Comments addressed

cmd/picoclaw/main.go

@@ -1304,16 +1304,7 @@ func skillsInstallFromRegistry(cfg *config.Config, registryName, slug string) {
 
 	registryMgr := skills.NewRegistryManagerFromConfig(skills.RegistryConfig{
 		MaxConcurrentSearches: cfg.Tools.Skills.MaxConcurrentSearches,
-		ClawHub: skills.ClawHubConfig{
+		ClawHub:               skills.ClawHubConfig(cfg.Tools.Skills.Registries.ClawHub),
-			Enabled:      cfg.Tools.Skills.Registries.ClawHub.Enabled,
-			BaseURL:      cfg.Tools.Skills.Registries.ClawHub.BaseURL,
-			AuthToken:    cfg.Tools.Skills.Registries.ClawHub.AuthToken,
-			SearchPath:   cfg.Tools.Skills.Registries.ClawHub.SearchPath,
-			SkillsPath:   cfg.Tools.Skills.Registries.ClawHub.SkillsPath,
-			DownloadPath: cfg.Tools.Skills.Registries.ClawHub.DownloadPath,
-			Timeout:      cfg.Tools.Skills.Registries.ClawHub.Timeout,
-			MaxZipSize:   cfg.Tools.Skills.Registries.ClawHub.MaxZipSize,
-		},
 	})
 
 	registry := registryMgr.GetRegistry(registryName)

pkg/agent/loop.go

@@ -108,16 +108,7 @@ func createToolRegistry(workspace string, restrict bool, cfg *config.Config, msg
 	// Skill discovery and installation tools
 	registryMgr := skills.NewRegistryManagerFromConfig(skills.RegistryConfig{
 		MaxConcurrentSearches: cfg.Tools.Skills.MaxConcurrentSearches,
-		ClawHub: skills.ClawHubConfig{
+		ClawHub:               skills.ClawHubConfig(cfg.Tools.Skills.Registries.ClawHub),
-			Enabled:      cfg.Tools.Skills.Registries.ClawHub.Enabled,
-			BaseURL:      cfg.Tools.Skills.Registries.ClawHub.BaseURL,
-			AuthToken:    cfg.Tools.Skills.Registries.ClawHub.AuthToken,
-			SearchPath:   cfg.Tools.Skills.Registries.ClawHub.SearchPath,
-			SkillsPath:   cfg.Tools.Skills.Registries.ClawHub.SkillsPath,
-			DownloadPath: cfg.Tools.Skills.Registries.ClawHub.DownloadPath,
-			Timeout:      cfg.Tools.Skills.Registries.ClawHub.Timeout,
-			MaxZipSize:   cfg.Tools.Skills.Registries.ClawHub.MaxZipSize,
-		},
 	})
 	searchCache := skills.NewSearchCache(cfg.Tools.Skills.SearchCache.MaxSize, time.Duration(cfg.Tools.Skills.SearchCache.TTLSeconds)*time.Second)
 	registry.Register(tools.NewFindSkillsTool(registryMgr, searchCache))

pkg/config/config.go

@@ -376,6 +376,11 @@ func DefaultConfig() *Config {
 						BaseURL: "https://clawhub.ai",
 					},
 				},
+				MaxConcurrentSearches: 2,
+				SearchCache: SearchCacheConfig{
+					MaxSize:    50,
+					TTLSeconds: 300,
+				},
 			},
 		},
 		Heartbeat: HeartbeatConfig{

pkg/skills/clawhub_registry.go

@@ -1,8 +1,6 @@
 package skills
 
 import (
-	"archive/zip"
-	"bytes"
 	"context"
 	"encoding/json"
 	"fmt"
@@ -10,9 +8,9 @@ import (
 	"net/http"
 	"net/url"
 	"os"
-	"path/filepath"
-	"strings"
 	"time"
+
+	"github.com/sipeed/picoclaw/pkg/utils"
 )
 
 const (
@@ -121,17 +119,17 @@ func (c *ClawHubRegistry) Search(ctx context.Context, query string, limit int) (
 
 	results := make([]SearchResult, 0, len(resp.Results))
 	for _, r := range resp.Results {
-		slug := derefStr(r.Slug, "")
+		slug := utils.DerefStr(r.Slug, "")
 		if slug == "" {
 			continue
 		}
 
-		summary := derefStr(r.Summary, "")
+		summary := utils.DerefStr(r.Summary, "")
 		if summary == "" {
 			continue
 		}
 
-		displayName := derefStr(r.DisplayName, "")
+		displayName := utils.DerefStr(r.DisplayName, "")
 		if displayName == "" {
 			displayName = slug
 		}
@@ -141,7 +139,7 @@ func (c *ClawHubRegistry) Search(ctx context.Context, query string, limit int) (
 			Slug:         slug,
 			DisplayName:  displayName,
 			Summary:      summary,
-			Version:      derefStr(r.Version, ""),
+			Version:      utils.DerefStr(r.Version, ""),
 			RegistryName: c.Name(),
 		})
 	}
@@ -169,8 +167,8 @@ type clawhubModerationInfo struct {
 }
 
 func (c *ClawHubRegistry) GetSkillMeta(ctx context.Context, slug string) (*SkillMeta, error) {
-	if !isSafeSlug(slug) {
+	if err := utils.ValidateSkillIdentifier(slug); err != nil {
-		return nil, fmt.Errorf("invalid slug: %q", slug)
+		return nil, fmt.Errorf("invalid slug %q: error: %s", slug, err.Error())
 	}
 
 	u := c.baseURL + c.skillsPath + "/" + url.PathEscape(slug)
@@ -209,8 +207,8 @@ func (c *ClawHubRegistry) GetSkillMeta(ctx context.Context, slug string) (*Skill
 // downloads the skill ZIP, and extracts it to targetDir.
 // Returns an InstallResult for the caller to use for moderation decisions.
 func (c *ClawHubRegistry) DownloadAndInstall(ctx context.Context, slug, version, targetDir string) (*InstallResult, error) {
-	if !isSafeSlug(slug) {
+	if err := utils.ValidateSkillIdentifier(slug); err != nil {
-		return nil, fmt.Errorf("invalid slug: %q", slug)
+		return nil, fmt.Errorf("invalid slug %q: error: %s", slug, err.Error())
 	}
 
 	// Step 1: Fetch metadata (with fallback).
@@ -237,7 +235,7 @@ func (c *ClawHubRegistry) DownloadAndInstall(ctx context.Context, slug, version,
 	}
 	result.Version = installVersion
 
-	// Step 3: Download ZIP.
+	// Step 3: Download ZIP to temp file (streams in ~32KB chunks).
 	u, err := url.Parse(c.baseURL + c.downloadPath)
 	if err != nil {
 		return nil, fmt.Errorf("invalid base URL: %w", err)
@@ -250,17 +248,22 @@ func (c *ClawHubRegistry) DownloadAndInstall(ctx context.Context, slug, version,
 	}
 	u.RawQuery = q.Encode()
 
-	zipData, err := c.doGet(ctx, u.String())
+	req, err := http.NewRequestWithContext(ctx, "GET", u.String(), nil)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create request: %w", err)
+	}
+	if c.authToken != "" {
+		req.Header.Set("Authorization", "Bearer "+c.authToken)
+	}
+
+	tmpPath, err := utils.DownloadToFile(ctx, c.client, req, int64(c.maxZipSize))
 	if err != nil {
 		return nil, fmt.Errorf("download failed: %w", err)
 	}
+	defer os.Remove(tmpPath)
 
-	if len(zipData) > c.maxZipSize {
+	// Step 4: Extract from file on disk.
-		return nil, fmt.Errorf("ZIP too large: %d bytes (max %d)", len(zipData), c.maxZipSize)
+	if err := utils.ExtractZipFile(tmpPath, targetDir); err != nil {
-	}
-
-	// Step 4: Extract.
-	if err := extractZip(zipData, targetDir); err != nil {
 		return nil, err
 	}
 
@@ -298,83 +301,3 @@ func (c *ClawHubRegistry) doGet(ctx context.Context, urlStr string) ([]byte, err
 
 	return body, nil
 }
-
-// --- ZIP extraction ---
-
-func extractZip(data []byte, targetDir string) error {
-	reader, err := zip.NewReader(bytes.NewReader(data), int64(len(data)))
-	if err != nil {
-		return fmt.Errorf("invalid ZIP: %w", err)
-	}
-
-	if err := os.MkdirAll(targetDir, 0755); err != nil {
-		return fmt.Errorf("failed to create target dir: %w", err)
-	}
-
-	for _, f := range reader.File {
-		// Path traversal protection.
-		cleanName := filepath.Clean(f.Name)
-		if strings.HasPrefix(cleanName, "..") || filepath.IsAbs(cleanName) {
-			return fmt.Errorf("zip entry has unsafe path: %q", f.Name)
-		}
-
-		destPath := filepath.Join(targetDir, cleanName)
-
-		// Double-check the resolved path is within target.
-		if !strings.HasPrefix(filepath.Clean(destPath), filepath.Clean(targetDir)) {
-			return fmt.Errorf("zip entry escapes target dir: %q", f.Name)
-		}
-
-		if f.FileInfo().IsDir() {
-			if err := os.MkdirAll(destPath, 0755); err != nil {
-				return err
-			}
-			continue
-		}
-
-		// Ensure parent directory exists.
-		if err := os.MkdirAll(filepath.Dir(destPath), 0755); err != nil {
-			return err
-		}
-
-		rc, err := f.Open()
-		if err != nil {
-			return fmt.Errorf("failed to open zip entry %q: %w", f.Name, err)
-		}
-
-		outFile, err := os.Create(destPath)
-		if err != nil {
-			rc.Close()
-			return fmt.Errorf("failed to create file %q: %w", destPath, err)
-		}
-
-		_, err = io.Copy(outFile, rc)
-		rc.Close()
-		outFile.Close()
-		if err != nil {
-			return fmt.Errorf("failed to extract %q: %w", f.Name, err)
-		}
-	}
-
-	return nil
-}
-
-// --- Utilities ---
-
-func isSafeSlug(slug string) bool {
-	slug = strings.TrimSpace(slug)
-	if slug == "" {
-		return false
-	}
-	if strings.ContainsAny(slug, "/\\") || strings.Contains(slug, "..") {
-		return false
-	}
-	return true
-}
-
-func derefStr(s *string, fallback string) string {
-	if s == nil {
-		return fallback
-	}
-	return *s
-}

pkg/skills/clawhub_registry_test.go

@@ -11,6 +11,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/sipeed/picoclaw/pkg/utils"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -159,8 +160,12 @@ func TestExtractZipPathTraversal(t *testing.T) {
 
 	zw.Close()
 
+	// Write to temp file for extractZipFile.
+	tmpZip := filepath.Join(t.TempDir(), "bad.zip")
+	require.NoError(t, os.WriteFile(tmpZip, buf.Bytes(), 0644))
+
 	tmpDir := t.TempDir()
-	err = extractZip(buf.Bytes(), tmpDir)
+	err = utils.ExtractZipFile(tmpZip, tmpDir)
 	assert.Error(t, err)
 	assert.Contains(t, err.Error(), "unsafe path")
 }
@@ -172,10 +177,14 @@ func TestExtractZipWithSubdirectories(t *testing.T) {
 		"examples/demo.yaml": "key: value",
 	})
 
+	// Write to temp file for extractZipFile.
+	tmpZip := filepath.Join(t.TempDir(), "test.zip")
+	require.NoError(t, os.WriteFile(tmpZip, zipBuf, 0644))
+
 	tmpDir := t.TempDir()
 	targetDir := filepath.Join(tmpDir, "my-skill")
 
-	err := extractZip(zipBuf, targetDir)
+	err := utils.ExtractZipFile(tmpZip, targetDir)
 	require.NoError(t, err)
 
 	// Verify nested file.

pkg/skills/registry.go

@@ -160,7 +160,7 @@ func (rm *RegistryManager) SearchAll(ctx context.Context, query string, limit in
 				return
 			}
 
-			searchCtx, cancel := context.WithTimeout(ctx, 15*time.Second)
+			searchCtx, cancel := context.WithTimeout(ctx, 1*time.Minute)
 			defer cancel()
 
 			results, err := r.Search(searchCtx, query, limit)
@@ -182,16 +182,18 @@ func (rm *RegistryManager) SearchAll(ctx context.Context, query string, limit in
 	var merged []SearchResult
 	var lastErr error
 
+	var anyRegistrySucceeded bool
 	for rr := range resultsCh {
 		if rr.err != nil {
 			lastErr = rr.err
 			continue
 		}
+		anyRegistrySucceeded = true
 		merged = append(merged, rr.results...)
 	}
 
 	// If all registries failed, return the last error.
-	if len(merged) == 0 && lastErr != nil {
+	if !anyRegistrySucceeded && lastErr != nil {
 		return nil, fmt.Errorf("all registries failed: %w", lastErr)
 	}
 

pkg/skills/registry_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/sipeed/picoclaw/pkg/utils"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -169,10 +170,10 @@ func TestSortByScoreDesc(t *testing.T) {
 }
 
 func TestIsSafeSlug(t *testing.T) {
-	assert.True(t, isSafeSlug("github"))
+	assert.NoError(t, utils.ValidateSkillIdentifier("github"))
-	assert.True(t, isSafeSlug("docker-compose"))
+	assert.NoError(t, utils.ValidateSkillIdentifier("docker-compose"))
-	assert.False(t, isSafeSlug(""))
+	assert.Error(t, utils.ValidateSkillIdentifier(""))
-	assert.False(t, isSafeSlug("../etc/passwd"))
+	assert.Error(t, utils.ValidateSkillIdentifier("../etc/passwd"))
-	assert.False(t, isSafeSlug("path/traversal"))
+	assert.Error(t, utils.ValidateSkillIdentifier("path/traversal"))
-	assert.False(t, isSafeSlug("path\\traversal"))
+	assert.Error(t, utils.ValidateSkillIdentifier("path\\traversal"))
 }

pkg/skills/search_cache.go

@@ -1,6 +1,7 @@
 package skills
 
 import (
+	"sort"
 	"strings"
 	"sync"
 	"time"
@@ -19,7 +20,7 @@ type SearchCache struct {
 
 type cacheEntry struct {
 	query     string
-	trigrams  map[string]struct{}
+	trigrams  []uint32
 	results   []SearchResult
 	createdAt time.Time
 }
@@ -53,12 +54,13 @@ func (sc *SearchCache) Get(query string) ([]SearchResult, bool) {
 		return nil, false
 	}
 
-	sc.mu.RLock()
+	sc.mu.Lock()
-	defer sc.mu.RUnlock()
+	defer sc.mu.Unlock()
 
 	// Exact match first.
 	if entry, ok := sc.entries[normalized]; ok {
 		if time.Since(entry.createdAt) < sc.ttl {
+			sc.moveToEndLocked(normalized)
 			return copyResults(entry.results), true
 		}
 	}
@@ -80,6 +82,7 @@ func (sc *SearchCache) Get(query string) ([]SearchResult, bool) {
 	}
 
 	if bestSim >= similarityThreshold && bestEntry != nil {
+		sc.moveToEndLocked(bestEntry.query)
 		return copyResults(bestEntry.results), true
 	}
 
@@ -166,39 +169,53 @@ func normalizeQuery(q string) string {
 	return strings.ToLower(strings.TrimSpace(q))
 }
 
-// buildTrigrams generates character trigrams from a string.
+// buildTrigrams generates hash of trigrams from a string.
 // Example: "hello" → {"hel", "ell", "llo"}
-func buildTrigrams(s string) map[string]struct{} {
+// "hel" -> 0x0068656c -> 4 bytes; compared to 16 byptes of a string
-	trigrams := make(map[string]struct{})
+func buildTrigrams(s string) []uint32 {
-	runes := []rune(s)
+	if len(s) < 3 {
-	for i := 0; i <= len(runes)-3; i++ {
+		return nil
-		tri := string(runes[i : i+3])
-		trigrams[tri] = struct{}{}
 	}
-	return trigrams
+
+	trigrams := make([]uint32, 0, len(s)-2)
+	for i := 0; i <= len(s)-3; i++ {
+		trigrams = append(trigrams, uint32(s[i])<<16|uint32(s[i+1])<<8|uint32(s[i+2]))
+	}
+
+	// Sort and Deduplication
+	sort.Slice(trigrams, func(i, j int) bool { return trigrams[i] < trigrams[j] })
+	n := 1
+	for i := 1; i < len(trigrams); i++ {
+		if trigrams[i] != trigrams[i-1] {
+			trigrams[n] = trigrams[i]
+			n++
+		}
+	}
+
+	return trigrams[:n]
 }
 
 // jaccardSimilarity computes |A ∩ B| / |A ∪ B|.
-func jaccardSimilarity(a, b map[string]struct{}) float64 {
+func jaccardSimilarity(a, b []uint32) float64 {
 	if len(a) == 0 && len(b) == 0 {
-		return 1.0
+		return 1
 	}
-	if len(a) == 0 || len(b) == 0 {
+	i, j := 0, 0
-		return 0.0
-	}
-
 	intersection := 0
-	for k := range a {
+
-		if _, ok := b[k]; ok {
+	for i < len(a) && j < len(b) {
+		if a[i] == b[j] {
 			intersection++
+			i++
+			j++
+		} else if a[i] < b[j] {
+			i++
+		} else {
+			j++
 		}
 	}
 
 	union := len(a) + len(b) - intersection
-	if union == 0 {
-		return 0.0
-	}
-
 	return float64(intersection) / float64(union)
 }
 

pkg/skills/search_cache_repro_test.go

@@ -0,0 +1,37 @@
+package skills
+
+import (
+	"testing"
+	"time"
+)
+
+func TestSearchCache_LRU_Behavior(t *testing.T) {
+	// Capacity 3
+	// Capacity 3
+	cache := NewSearchCache(3, time.Hour)
+
+	// Fill cache: query-A, query-B, query-C
+	// Use longer strings to ensure trigrams are generated and avoid false positive similarity
+	cache.Put("query-A", []SearchResult{{Slug: "A"}})
+	cache.Put("query-B", []SearchResult{{Slug: "B"}})
+	cache.Put("query-C", []SearchResult{{Slug: "C"}})
+
+	// Access query-A (should make it most recently used)
+	// Current behavior: query-A remains at front (oldest) if Get doesn't update order
+	if _, found := cache.Get("query-A"); !found {
+		t.Fatal("query-A should be in cache")
+	}
+
+	// Add query-D. Should evict query-A if FIFO (current bug), or query-B if LRU (desired).
+	cache.Put("query-D", []SearchResult{{Slug: "D"}})
+
+	// Check if query-A is still there
+	if _, found := cache.Get("query-A"); !found {
+		t.Fatalf("query-A was evicted! valid LRU should have kept query-A and evicted query-B.")
+	}
+
+	// Check if query-B is evicted (if A was kept, B should be gone)
+	if _, found := cache.Get("query-B"); found {
+		t.Fatal("query-B should have been evicted")
+	}
+}

pkg/skills/search_cache_test.go

@@ -120,9 +120,9 @@ func TestSearchCacheResultsCopied(t *testing.T) {
 
 func TestBuildTrigrams(t *testing.T) {
 	trigrams := buildTrigrams("hello")
-	assert.Contains(t, trigrams, "hel")
+	assert.Contains(t, trigrams, uint32('h')<<16|uint32('e')<<8|uint32('l'))
-	assert.Contains(t, trigrams, "ell")
+	assert.Contains(t, trigrams, uint32('e')<<16|uint32('l')<<8|uint32('l'))
-	assert.Contains(t, trigrams, "llo")
+	assert.Contains(t, trigrams, uint32('l')<<16|uint32('l')<<8|uint32('o'))
 	assert.Len(t, trigrams, 3)
 }
 
@@ -168,5 +168,33 @@ func TestSearchCacheConcurrency(t *testing.T) {
 	}()
 
 	<-done
-	<-done
+}
+
+func TestSearchCacheLRUUpdateOnGet(t *testing.T) {
+	// Capacity 3
+	cache := NewSearchCache(3, time.Hour)
+
+	// Fill cache: query-A, query-B, query-C
+	// Use longer strings to ensure trigrams are generated and avoid false positive similarity
+	cache.Put("query-A", []SearchResult{{Slug: "A"}})
+	cache.Put("query-B", []SearchResult{{Slug: "B"}})
+	cache.Put("query-C", []SearchResult{{Slug: "C"}})
+
+	// Access query-A (should make it most recently used)
+	if _, found := cache.Get("query-A"); !found {
+		t.Fatal("query-A should be in cache")
+	}
+
+	// Add query-D. Should evict query-B (LRU) instead of query-A (which was refreshed)
+	cache.Put("query-D", []SearchResult{{Slug: "D"}})
+
+	// Check if query-A is still there
+	if _, found := cache.Get("query-A"); !found {
+		t.Fatalf("query-A was evicted! valid LRU should have kept query-A and evicted query-B.")
+	}
+
+	// Check if query-B is evicted
+	if _, found := cache.Get("query-B"); found {
+		t.Fatal("query-B should have been evicted")
+	}
 }

pkg/tools/skills_install.go

@@ -6,10 +6,12 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"strings"
+	"sync"
 	"time"
 
+	"github.com/sipeed/picoclaw/pkg/logger"
 	"github.com/sipeed/picoclaw/pkg/skills"
+	"github.com/sipeed/picoclaw/pkg/utils"
 )
 
 // InstallSkillTool allows the LLM agent to install skills from registries.
@@ -64,25 +66,25 @@ func (t *InstallSkillTool) Parameters() map[string]interface{} {
 }
 
 func (t *InstallSkillTool) Execute(ctx context.Context, args map[string]interface{}) *ToolResult {
-	slug, ok := args["slug"].(string)
+	// Install lock to prevent concurrent directory operations.
-	if !ok || strings.TrimSpace(slug) == "" {
+	// Ideally this should be done at a `slug` level, currently, its at a `workspace` level.
-		return ErrorResult("slug is required and must be a non-empty string")
+	slugLock := sync.Mutex{}
+	slugLock.Lock()
+	defer slugLock.Unlock()
+
+	// Validate slug
+	slug, _ := args["slug"].(string)
+	if err := utils.ValidateSkillIdentifier(slug); err != nil {
+		return ErrorResult(fmt.Sprintf("invalid slug %q: error: %s", slug, err.Error()))
 	}
 
-	slug = strings.TrimSpace(slug)
+	// Validate registry
-
+	registryName, _ := args["registry"].(string)
-	// Validate slug safety.
+	if err := utils.ValidateSkillIdentifier(registryName); err != nil {
-	if strings.ContainsAny(slug, "/\\") || strings.Contains(slug, "..") {
+		return ErrorResult(fmt.Sprintf("invalid registry %q: error: %s", registryName, err.Error()))
-		return ErrorResult(fmt.Sprintf("invalid slug: %q (must not contain path separators or '..')", slug))
 	}
 
 	version, _ := args["version"].(string)
-	registryName, ok := args["registry"].(string)
-	if !ok || strings.TrimSpace(registryName) == "" {
-		return ErrorResult("registry is required")
-	}
-	registryName = strings.TrimSpace(registryName)
-
 	force, _ := args["force"].(bool)
 
 	// Check if already installed.
@@ -125,7 +127,15 @@ func (t *InstallSkillTool) Execute(ctx context.Context, args map[string]interfac
 
 	// Write origin metadata.
 	if err := writeOriginMeta(targetDir, registry.Name(), slug, result.Version); err != nil {
-		// Non-fatal: skill is installed, just origin tracking failed.
+		logger.ErrorCF("tool", "Failed to write origin metadata",
+			map[string]interface{}{
+				"tool":     "install_skill",
+				"error":    err.Error(),
+				"target":   targetDir,
+				"registry": registry.Name(),
+				"slug":     slug,
+				"version":  result.Version,
+			})
 		_ = err
 	}
 

pkg/tools/skills_search.go

@@ -53,7 +53,8 @@ func (t *FindSkillsTool) Parameters() map[string]interface{} {
 
 func (t *FindSkillsTool) Execute(ctx context.Context, args map[string]interface{}) *ToolResult {
 	query, ok := args["query"].(string)
-	if !ok || strings.TrimSpace(query) == "" {
+	query = strings.ToLower(strings.TrimSpace(query))
+	if !ok || query == "" {
 		return ErrorResult("query is required and must be a non-empty string")
 	}
 

pkg/utils/download.go

@@ -0,0 +1,93 @@
+package utils
+
+import (
+	"context"
+	"fmt"
+	"io"
+	"net/http"
+	"os"
+
+	"github.com/sipeed/picoclaw/pkg/logger"
+)
+
+// DownloadToFile streams an HTTP response body to a temporary file in small
+// chunks (~32KB), keeping peak memory usage constant regardless of file size.
+//
+// Parameters:
+//   - ctx:      context for cancellation/timeout
+//   - client:   HTTP client to use (caller controls timeouts, transport, etc.)
+//   - req:      fully prepared *http.Request (method, URL, headers, etc.)
+//   - maxBytes: maximum bytes to download; 0 means no limit
+//
+// Returns the path to the temporary file. The caller is responsible for
+// removing it when done (defer os.Remove(path)).
+//
+// On any error the temp file is cleaned up automatically.
+func DownloadToFile(ctx context.Context, client *http.Client, req *http.Request, maxBytes int64) (string, error) {
+	// Attach context.
+	req = req.WithContext(ctx)
+
+	logger.DebugCF("download", "Starting download", map[string]interface{}{
+		"url":       req.URL.String(),
+		"max_bytes": maxBytes,
+	})
+
+	resp, err := client.Do(req)
+	if err != nil {
+		return "", fmt.Errorf("request failed: %w", err)
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+		// Read a small amount for the error message.
+		errBody := make([]byte, 512)
+		n, _ := io.ReadFull(resp.Body, errBody)
+		return "", fmt.Errorf("HTTP %d: %s", resp.StatusCode, string(errBody[:n]))
+	}
+
+	// Create temp file.
+	tmpFile, err := os.CreateTemp("", "picoclaw-dl-*")
+	if err != nil {
+		return "", fmt.Errorf("failed to create temp file: %w", err)
+	}
+	tmpPath := tmpFile.Name()
+
+	logger.DebugCF("download", "Streaming to temp file", map[string]interface{}{
+		"path": tmpPath,
+	})
+
+	// Cleanup helper — removes the temp file on any error.
+	cleanup := func() {
+		_ = tmpFile.Close()
+		_ = os.Remove(tmpPath)
+	}
+
+	// Optionally limit the download size.
+	var src io.Reader = resp.Body
+	if maxBytes > 0 {
+		src = io.LimitReader(resp.Body, maxBytes+1) // +1 to detect overflow
+	}
+
+	written, err := io.Copy(tmpFile, src)
+	if err != nil {
+		cleanup()
+		return "", fmt.Errorf("download write failed: %w", err)
+	}
+
+	if maxBytes > 0 && written > maxBytes {
+		cleanup()
+		return "", fmt.Errorf("download too large: %d bytes (max %d)", written, maxBytes)
+	}
+
+	if err := tmpFile.Close(); err != nil {
+		_ = os.Remove(tmpPath)
+		return "", fmt.Errorf("failed to close temp file: %w", err)
+	}
+
+	logger.DebugCF("download", "Download complete", map[string]interface{}{
+		"path":          tmpPath,
+		"bytes_written": written,
+	})
+
+	return tmpPath, nil
+}

pkg/utils/skills.go

@@ -0,0 +1,18 @@
+package utils
+
+import (
+	"fmt"
+	"strings"
+)
+
+// 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 == "" {
+		return fmt.Errorf("identifier is required and must be a non-empty string")
+	}
+	if strings.ContainsAny(identifier, "/\\") || strings.Contains(identifier, "..") {
+		return fmt.Errorf("identifier must not contain path separators or '..' to prevent directory traversal")
+	}
+	return nil
+}

pkg/utils/string.go

@@ -14,3 +14,12 @@ func Truncate(s string, maxLen int) string {
 	}
 	return string(runes[:maxLen-3]) + "..."
 }
+
+// DerefStr dereferences a pointer to a string and
+// returns the value or a fallback if the pointer is nil.
+func DerefStr(s *string, fallback string) string {
+	if s == nil {
+		return fallback
+	}
+	return *s
+}

pkg/utils/zip.go

@@ -0,0 +1,101 @@
+package utils
+
+import (
+	"archive/zip"
+	"fmt"
+	"io"
+	"os"
+	"path/filepath"
+	"strings"
+
+	"github.com/sipeed/picoclaw/pkg/logger"
+)
+
+// ExtractZipFile extracts a ZIP archive from disk to targetDir.
+// It reads entries one at a time from disk, keeping memory usage minimal.
+//
+// Security: rejects path traversal attempts and symlinks.
+func ExtractZipFile(zipPath string, targetDir string) error {
+	reader, err := zip.OpenReader(zipPath)
+	if err != nil {
+		return fmt.Errorf("invalid ZIP: %w", err)
+	}
+	defer reader.Close()
+
+	logger.DebugCF("zip", "Extracting ZIP", map[string]interface{}{
+		"zip_path":   zipPath,
+		"target_dir": targetDir,
+		"entries":    len(reader.File),
+	})
+
+	if err := os.MkdirAll(targetDir, 0755); err != nil {
+		return fmt.Errorf("failed to create target dir: %w", err)
+	}
+
+	for _, f := range reader.File {
+		// Path traversal protection.
+		cleanName := filepath.Clean(f.Name)
+		if strings.HasPrefix(cleanName, "..") || filepath.IsAbs(cleanName) {
+			return fmt.Errorf("zip entry has unsafe path: %q", f.Name)
+		}
+
+		destPath := filepath.Join(targetDir, cleanName)
+
+		// Double-check the resolved path is within target.
+		if !strings.HasPrefix(filepath.Clean(destPath), filepath.Clean(targetDir)) {
+			return fmt.Errorf("zip entry escapes target dir: %q", f.Name)
+		}
+
+		mode := f.FileInfo().Mode()
+
+		// Reject any symlink.
+		if mode&os.ModeSymlink != 0 {
+			return fmt.Errorf("zip contains symlink %q; symlinks are not allowed", f.Name)
+		}
+
+		if f.FileInfo().IsDir() {
+			if err := os.MkdirAll(destPath, 0755); err != nil {
+				return err
+			}
+			continue
+		}
+
+		// Ensure parent directory exists.
+		if err := os.MkdirAll(filepath.Dir(destPath), 0755); err != nil {
+			return err
+		}
+
+		if err := extractSingleFile(f, destPath); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// extractSingleFile extracts one zip.File entry to destPath.
+func extractSingleFile(f *zip.File, destPath string) error {
+	rc, err := f.Open()
+	if err != nil {
+		return fmt.Errorf("failed to open zip entry %q: %w", f.Name, err)
+	}
+	defer rc.Close()
+
+	outFile, err := os.Create(destPath)
+	if err != nil {
+		return fmt.Errorf("failed to create file %q: %w", destPath, err)
+	}
+
+	if _, err := io.Copy(outFile, rc); err != nil {
+		_ = outFile.Close()
+		_ = os.Remove(destPath)
+		return fmt.Errorf("failed to extract %q: %w", f.Name, err)
+	}
+
+	if err := outFile.Close(); err != nil {
+		_ = os.Remove(destPath)
+		return fmt.Errorf("failed to close file %q: %w", destPath, err)
+	}
+
+	return nil
+}