diff --git a/cmd/picoclaw/cmd_skills.go b/cmd/picoclaw/cmd_skills.go index 9a71a944c..f156f92d7 100644 --- a/cmd/picoclaw/cmd_skills.go +++ b/cmd/picoclaw/cmd_skills.go @@ -6,6 +6,7 @@ package main import ( "context" "fmt" + "net/url" "os" "path/filepath" "strings" @@ -72,6 +73,7 @@ func skillsHelp() { fmt.Println("\nSkills commands:") fmt.Println(" list List installed skills") fmt.Println(" install [subpath] Install skill from GitHub (repo: owner/repo or owner/repo@branch; subpath e.g. skills/kanban-ai)") + fmt.Println(" install [name] Install from .zip / .tar.gz / .tgz (file path or URL)") fmt.Println(" reinstall [subpath] Overwrite existing skill (same args as install)") fmt.Println(" install-builtin Install all builtin skills to workspace") fmt.Println(" list-builtin List available builtin skills") @@ -82,10 +84,9 @@ func skillsHelp() { fmt.Println("Examples:") fmt.Println(" picoclaw skills list") fmt.Println(" picoclaw skills install sipeed/picoclaw-skills") - fmt.Println(" picoclaw skills install sipeed/picoclaw-skills@test") fmt.Println(" picoclaw skills install sipeed/picoclaw-skills weather") - fmt.Println(" picoclaw skills reinstall sipeed/picoclaw-skills k8s-report") - fmt.Println(" picoclaw skills install-builtin") + fmt.Println(" picoclaw skills install ./skill.zip") + fmt.Println(" picoclaw skills install https://example.com/skill.tar.gz my-skill") fmt.Println(" picoclaw skills install --registry clawhub github") fmt.Println(" picoclaw skills remove weather") } @@ -108,6 +109,39 @@ func skillsListCmd(loader *skills.SkillsLoader) { } } +// isArchiveInstallArg returns true when arg looks like a path or URL to an archive (.zip, .tar.gz, .tgz), +// so we treat it as archive install and avoid conflicting with GitHub repo names like owner/repo.zip. +func isArchiveInstallArg(arg string) bool { + arg = strings.TrimSpace(arg) + if arg == "" { + return false + } + // Extension: path or URL path must end with .zip, .tar.gz, or .tgz. + base := arg + if u, err := url.Parse(arg); err == nil && (u.Scheme == "http" || u.Scheme == "https") { + base = filepath.Base(u.Path) + } else { + base = filepath.Base(arg) + } + lower := strings.ToLower(base) + hasExt := strings.HasSuffix(lower, ".zip") || strings.HasSuffix(lower, ".tar.gz") || strings.HasSuffix(lower, ".tgz") + if !hasExt { + return false + } + // Path or protocol: starts with ./, /, \, or http(s)://, or is an existing file. + if strings.HasPrefix(arg, "./") || strings.HasPrefix(arg, "/") || strings.HasPrefix(arg, "\\") { + return true + } + if strings.HasPrefix(strings.ToLower(arg), "http://") || strings.HasPrefix(strings.ToLower(arg), "https://") { + return true + } + info, err := os.Stat(arg) + if err == nil && info.Mode().IsRegular() { + return true + } + return false +} + func skillsInstallCmd(installer *skills.SkillInstaller, cfg *config.Config, force bool) { if len(os.Args) < 4 { verb := "install" @@ -115,6 +149,7 @@ func skillsInstallCmd(installer *skills.SkillInstaller, cfg *config.Config, forc verb = "reinstall" } fmt.Printf("Usage: picoclaw skills %s [subpath]\n", verb) + fmt.Printf(" picoclaw skills %s [name] (for .zip / .tar.gz / .tgz)\n", verb) fmt.Println(" picoclaw skills install --registry ") fmt.Println(" repo: owner/repo or owner/repo@branch (branch defaults to main)") fmt.Println(" subpath: optional path in repo (e.g. weather or skills/kanban-ai)") @@ -134,6 +169,29 @@ func skillsInstallCmd(installer *skills.SkillInstaller, cfg *config.Config, forc return } + // Archive path or URL: .zip / .tar.gz / .tgz (path or URL only, to avoid conflict with GitHub repo names). + arg := os.Args[3] + if isArchiveInstallArg(arg) { + var skillName string + if len(os.Args) >= 5 { + skillName = strings.TrimSpace(os.Args[4]) + } + if force { + fmt.Printf("Reinstalling skill from %s...\n", arg) + } else { + fmt.Printf("Installing skill from %s...\n", arg) + } + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + installedName, err := installer.InstallFromArchive(ctx, arg, skillName, force) + if err != nil { + fmt.Printf("\u2717 Failed to install skill: %v\n", err) + os.Exit(1) + } + fmt.Printf("\u2713 Skill '%s' installed successfully!\n", installedName) + return + } + // GitHub path: [subpath] spec := os.Args[3] repo, branch, err := skills.ParseInstallSpec(spec) diff --git a/docs/skills-cli.md b/docs/skills-cli.md index d1dacdc59..bf636e0db 100644 --- a/docs/skills-cli.md +++ b/docs/skills-cli.md @@ -11,7 +11,7 @@ Some features (e.g. `reinstall`, `repo@branch`, optional `subpath`) may require | Subcommand | Usage | Description | |------------|--------|-------------| | list | `picoclaw skills list` | List installed skills | -| install | `picoclaw skills install [subpath]` or `install --registry ` | Install from GitHub or a registry | +| install | `picoclaw skills install [subpath]`, `install [name]`, or `install --registry ` | Install from GitHub, from a .zip/.tar.gz/.tgz file or URL, or from a registry | | reinstall | `picoclaw skills reinstall [subpath]` | Overwrite install (remove then install) | | install-builtin | `picoclaw skills install-builtin` | Copy built-in skills into the workspace | | list-builtin | `picoclaw skills list-builtin` | List available built-in skills | @@ -59,6 +59,34 @@ picoclaw skills reinstall sipeed/picoclaw-skills k8s-report --- +## Install from archive (zip / tar.gz / tgz) + +You can install a skill from a **local file** or **HTTP(S) URL** that points to a `.zip`, `.tar.gz`, or `.tgz` archive. The archive must contain a `SKILL.md` at the root (or as the only file under a single top-level directory, which is normalized automatically). + +**Usage:** `picoclaw skills install [name]` + +- **path-or-url** — A local path (e.g. `./skill.zip`, `/tmp/skill.tar.gz`) or a URL (e.g. `https://example.com/skill.zip`). To avoid conflicting with GitHub repo names like `owner/repo.zip`, the installer treats an argument as an archive only when it **looks like a path or URL** (starts with `./`, `/`, `\`, or `http://`/`https://`) **and** has an archive extension. +- **name** — Optional. The skill name under `workspace/skills/`. If omitted, it is derived from the file or URL name (e.g. `skill.zip` → `skill`). + +**Supported formats:** `.zip`, `.tar.gz`, `.tgz`. + +**Reinstall:** Use `picoclaw skills reinstall [name]` to overwrite an existing skill installed from an archive. + +**Examples:** + +```bash +# Install from a local zip file (skill name = my-skill from filename) +picoclaw skills install ./my-skill.zip + +# Install from a URL and set skill name explicitly +picoclaw skills install https://example.com/skill.tar.gz my-skill + +# Overwrite existing install +picoclaw skills reinstall ./skill.zip +``` + +--- + ## Install from Registry Use a configured registry (e.g. ClawHub) to install by slug. diff --git a/pkg/skills/installer.go b/pkg/skills/installer.go index f2a2f04fe..25c720e1e 100644 --- a/pkg/skills/installer.go +++ b/pkg/skills/installer.go @@ -6,10 +6,13 @@ import ( "fmt" "io" "net/http" + "net/url" "os" "path/filepath" "strings" "time" + + "github.com/sipeed/picoclaw/pkg/utils" ) type SkillInstaller struct { @@ -331,6 +334,157 @@ func (si *SkillInstaller) InstallFromGitHub(ctx context.Context, spec string) er return err } +const maxArchiveDownloadBytes = 50 * 1024 * 1024 // 50MB + +// archiveExt returns the archive extension from a path or URL (e.g. ".zip", ".tar.gz", ".tgz"), or "". +func archiveExt(pathOrURL string) string { + base := pathOrURL + if u, err := url.Parse(pathOrURL); err == nil && (u.Scheme == "http" || u.Scheme == "https") { + base = filepath.Base(u.Path) + } else { + base = filepath.Base(pathOrURL) + } + base = strings.ToLower(base) + if strings.HasSuffix(base, ".tar.gz") { + return ".tar.gz" + } + if strings.HasSuffix(base, ".tgz") { + return ".tgz" + } + if strings.HasSuffix(base, ".zip") { + return ".zip" + } + return "" +} + +// skillNameFromArchivePath derives skill name from path or URL by stripping archive extension. +func skillNameFromArchivePath(pathOrURL string) string { + base := pathOrURL + if u, err := url.Parse(pathOrURL); err == nil && (u.Scheme == "http" || u.Scheme == "https") { + base = filepath.Base(u.Path) + } else { + base = filepath.Base(pathOrURL) + } + base = strings.TrimSuffix(strings.ToLower(base), ".tar.gz") + base = strings.TrimSuffix(base, ".tgz") + base = strings.TrimSuffix(base, ".zip") + return base +} + +// InstallFromArchive installs a skill from a .zip, .tar.gz, or .tgz file (local path or http(s) URL). +// If skillName is empty, it is derived from the archive path/URL. If force is true, existing skill dir is removed first. +func (si *SkillInstaller) InstallFromArchive(ctx context.Context, archivePathOrURL, skillName string, force bool) (string, error) { + archivePathOrURL = strings.TrimSpace(archivePathOrURL) + if archivePathOrURL == "" { + return "", fmt.Errorf("empty archive path or URL") + } + + ext := archiveExt(archivePathOrURL) + if ext == "" { + return "", fmt.Errorf("unsupported archive format (need .zip, .tar.gz, or .tgz)") + } + + var localPath string + isURL := strings.HasPrefix(strings.ToLower(archivePathOrURL), "http://") || strings.HasPrefix(strings.ToLower(archivePathOrURL), "https://") + if isURL { + req, err := http.NewRequestWithContext(ctx, "GET", archivePathOrURL, nil) + if err != nil { + return "", fmt.Errorf("failed to create request: %w", err) + } + client := &http.Client{Timeout: 60 * time.Second} + tmpPath, err := utils.DownloadToFile(ctx, client, req, maxArchiveDownloadBytes) + if err != nil { + return "", fmt.Errorf("download failed: %w", err) + } + defer os.Remove(tmpPath) + localPath = tmpPath + } else { + if _, err := os.Stat(archivePathOrURL); err != nil { + return "", fmt.Errorf("archive file not found: %w", err) + } + localPath = archivePathOrURL + } + + if skillName == "" { + skillName = skillNameFromArchivePath(archivePathOrURL) + } + if skillName == "" { + return "", fmt.Errorf("could not derive skill name from path or URL") + } + + skillDir := filepath.Join(si.workspace, "skills", skillName) + if _, err := os.Stat(skillDir); err == nil { + if !force { + return "", fmt.Errorf("skill '%s' already exists (use reinstall to overwrite)", skillName) + } + if err := os.RemoveAll(skillDir); err != nil { + return "", fmt.Errorf("failed to remove existing skill: %w", err) + } + } + + if err := os.MkdirAll(skillDir, 0755); err != nil { + return "", fmt.Errorf("failed to create skill directory: %w", err) + } + + switch ext { + case ".zip": + if err := utils.ExtractZipFile(localPath, skillDir); err != nil { + os.RemoveAll(skillDir) + return "", err + } + case ".tar.gz", ".tgz": + if err := utils.ExtractTarGzFile(localPath, skillDir); err != nil { + os.RemoveAll(skillDir) + return "", err + } + default: + os.RemoveAll(skillDir) + return "", fmt.Errorf("unsupported archive format: %s", ext) + } + + // Normalize: if skillDir contains only one directory and SKILL.md is not at root, move contents up. + if err := normalizeSingleRootDir(skillDir); err != nil { + os.RemoveAll(skillDir) + return "", err + } + + skillMD := filepath.Join(skillDir, "SKILL.md") + if _, err := os.Stat(skillMD); err != nil { + os.RemoveAll(skillDir) + return "", fmt.Errorf("SKILL.md not found in archive (invalid skill package)") + } + + return skillName, nil +} + +// normalizeSingleRootDir moves the contents of a single top-level directory up into skillDir and removes that directory. +func normalizeSingleRootDir(skillDir string) error { + entries, err := os.ReadDir(skillDir) + if err != nil { + return err + } + if len(entries) != 1 || !entries[0].IsDir() { + return nil + } + skillMD := filepath.Join(skillDir, "SKILL.md") + if _, err := os.Stat(skillMD); err == nil { + return nil + } + inner := filepath.Join(skillDir, entries[0].Name()) + innerEntries, err := os.ReadDir(inner) + if err != nil { + return err + } + for _, e := range innerEntries { + src := filepath.Join(inner, e.Name()) + dst := filepath.Join(skillDir, e.Name()) + if err := os.Rename(src, dst); err != nil { + return fmt.Errorf("move %q to skill root: %w", e.Name(), err) + } + } + return os.Remove(inner) +} + func (si *SkillInstaller) Uninstall(skillName string) error { skillDir := filepath.Join(si.workspace, "skills", skillName) diff --git a/pkg/skills/installer_test.go b/pkg/skills/installer_test.go index f129c975d..f5011621e 100644 --- a/pkg/skills/installer_test.go +++ b/pkg/skills/installer_test.go @@ -1,6 +1,8 @@ package skills import ( + "archive/zip" + "bytes" "context" "net/http" "net/http/httptest" @@ -12,6 +14,20 @@ import ( "github.com/stretchr/testify/require" ) +func createZipWithSkill(t *testing.T, files map[string]string) []byte { + t.Helper() + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + for name, content := range files { + w, err := zw.Create(name) + require.NoError(t, err) + _, err = w.Write([]byte(content)) + require.NoError(t, err) + } + require.NoError(t, zw.Close()) + return buf.Bytes() +} + func TestParseInstallSpec(t *testing.T) { tests := []struct { spec string @@ -95,3 +111,62 @@ func TestInstallFromGitHubEx_already_exists(t *testing.T) { assert.Contains(t, err.Error(), "already exists") assert.Contains(t, err.Error(), "reinstall") } + +func TestInstallFromArchive_zip(t *testing.T) { + zipData := createZipWithSkill(t, map[string]string{ + "SKILL.md": "# Archive Skill", + }) + zipPath := filepath.Join(t.TempDir(), "skill.zip") + require.NoError(t, os.WriteFile(zipPath, zipData, 0o644)) + + dir := t.TempDir() + si := NewSkillInstaller(dir) + ctx := context.Background() + + skillName, err := si.InstallFromArchive(ctx, zipPath, "", false) + require.NoError(t, err) + assert.Equal(t, "skill", skillName) + + content, err := os.ReadFile(filepath.Join(dir, "skills", "skill", "SKILL.md")) + require.NoError(t, err) + assert.Equal(t, "# Archive Skill", string(content)) +} + +func TestInstallFromArchive_zip_singleRootDir_normalized(t *testing.T) { + // Archive with single top-level dir (e.g. my-skill/SKILL.md) should be normalized so SKILL.md is at skillDir root. + zipData := createZipWithSkill(t, map[string]string{ + "my-skill/SKILL.md": "# Normalized Skill", + }) + zipPath := filepath.Join(t.TempDir(), "my-skill.zip") + require.NoError(t, os.WriteFile(zipPath, zipData, 0o644)) + + dir := t.TempDir() + si := NewSkillInstaller(dir) + ctx := context.Background() + + skillName, err := si.InstallFromArchive(ctx, zipPath, "my-skill", false) + require.NoError(t, err) + assert.Equal(t, "my-skill", skillName) + + content, err := os.ReadFile(filepath.Join(dir, "skills", "my-skill", "SKILL.md")) + require.NoError(t, err) + assert.Equal(t, "# Normalized Skill", string(content)) +} + +func TestInstallFromArchive_already_exists(t *testing.T) { + zipData := createZipWithSkill(t, map[string]string{"SKILL.md": "# Skill"}) + zipPath := filepath.Join(t.TempDir(), "dup.zip") + require.NoError(t, os.WriteFile(zipPath, zipData, 0o644)) + + dir := t.TempDir() + si := NewSkillInstaller(dir) + ctx := context.Background() + + _, err := si.InstallFromArchive(ctx, zipPath, "dup", false) + require.NoError(t, err) + + _, err = si.InstallFromArchive(ctx, zipPath, "dup", false) + require.Error(t, err) + assert.Contains(t, err.Error(), "already exists") + assert.Contains(t, err.Error(), "reinstall") +} diff --git a/pkg/utils/tar.go b/pkg/utils/tar.go new file mode 100644 index 000000000..a58bb2e2d --- /dev/null +++ b/pkg/utils/tar.go @@ -0,0 +1,105 @@ +package utils + +import ( + "archive/tar" + "compress/gzip" + "fmt" + "io" + "os" + "path/filepath" + "strings" + + "github.com/sipeed/picoclaw/pkg/logger" +) + +const maxTarFileSize = 5 * 1024 * 1024 // 5MB, same as zip + +// ExtractTarGzFile extracts a gzip-compressed tar archive from disk to targetDir. +// Security: rejects path traversal (.., absolute paths), symlinks, and limits single-file size. +func ExtractTarGzFile(tarGzPath string, targetDir string) error { + f, err := os.Open(tarGzPath) + if err != nil { + return fmt.Errorf("open tar.gz: %w", err) + } + defer f.Close() + + gzr, err := gzip.NewReader(f) + if err != nil { + return fmt.Errorf("invalid gzip: %w", err) + } + defer gzr.Close() + + tr := tar.NewReader(gzr) + return extractTarReader(tr, targetDir, tarGzPath) +} + +func extractTarReader(tr *tar.Reader, targetDir string, srcLabel string) error { + if err := os.MkdirAll(targetDir, 0o755); err != nil { + return fmt.Errorf("failed to create target dir: %w", err) + } + + targetDirClean := filepath.Clean(targetDir) + + for { + hdr, err := tr.Next() + if err == io.EOF { + break + } + if err != nil { + return fmt.Errorf("read tar: %w", err) + } + + cleanName := filepath.Clean(hdr.Name) + if strings.HasPrefix(cleanName, "..") || filepath.IsAbs(cleanName) { + _, _ = io.CopyN(io.Discard, tr, hdr.Size) + return fmt.Errorf("tar entry has unsafe path: %q", hdr.Name) + } + + destPath := filepath.Join(targetDir, cleanName) + if !strings.HasPrefix(filepath.Clean(destPath), targetDirClean+string(filepath.Separator)) && + filepath.Clean(destPath) != targetDirClean { + _, _ = io.CopyN(io.Discard, tr, hdr.Size) + return fmt.Errorf("tar entry escapes target dir: %q", hdr.Name) + } + + switch hdr.Typeflag { + case tar.TypeSymlink, tar.TypeLink: + return fmt.Errorf("tar contains link %q; symlinks/hardlinks are not allowed", hdr.Name) + case tar.TypeDir: + if err := os.MkdirAll(destPath, 0o755); err != nil { + return err + } + continue + case tar.TypeReg, tar.TypeRegA: + // regular file + default: + // skip other types (char, block, fifo, etc.) + continue + } + + if err := os.MkdirAll(filepath.Dir(destPath), 0o755); err != nil { + return err + } + + outFile, err := os.Create(destPath) + if err != nil { + return fmt.Errorf("create %q: %w", destPath, err) + } + written, err := io.CopyN(outFile, tr, maxTarFileSize+1) + outFile.Close() + if err != nil && err != io.EOF { + os.Remove(destPath) + return fmt.Errorf("extract %q: %w", hdr.Name, err) + } + if written > maxTarFileSize { + os.Remove(destPath) + return fmt.Errorf("tar entry %q exceeds max size (%d bytes)", hdr.Name, written) + } + } + + logger.DebugCF("tar", "Extracted tar.gz", map[string]any{ + "src": srcLabel, + "target_dir": targetDir, + }) + return nil +} diff --git a/pkg/utils/tar_test.go b/pkg/utils/tar_test.go new file mode 100644 index 000000000..52df9dc0a --- /dev/null +++ b/pkg/utils/tar_test.go @@ -0,0 +1,85 @@ +package utils + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func createTestTarGz(t *testing.T, files map[string]string) []byte { + t.Helper() + var buf bytes.Buffer + gw := gzip.NewWriter(&buf) + tw := tar.NewWriter(gw) + for name, content := range files { + err := tw.WriteHeader(&tar.Header{Name: name, Size: int64(len(content)), Mode: 0o644}) + require.NoError(t, err) + _, err = tw.Write([]byte(content)) + require.NoError(t, err) + } + require.NoError(t, tw.Close()) + require.NoError(t, gw.Close()) + return buf.Bytes() +} + +func TestExtractTarGzFile_valid(t *testing.T) { + data := createTestTarGz(t, map[string]string{ + "SKILL.md": "# Test Skill", + "extra.txt": "hello", + }) + tmpZip := filepath.Join(t.TempDir(), "skill.tar.gz") + require.NoError(t, os.WriteFile(tmpZip, data, 0o644)) + + targetDir := t.TempDir() + err := ExtractTarGzFile(tmpZip, targetDir) + require.NoError(t, err) + + skillMD := filepath.Join(targetDir, "SKILL.md") + content, err := os.ReadFile(skillMD) + require.NoError(t, err) + require.Equal(t, "# Test Skill", string(content)) + extra, _ := os.ReadFile(filepath.Join(targetDir, "extra.txt")) + require.Equal(t, "hello", string(extra)) +} + +func TestExtractTarGzFile_pathTraversal_rejected(t *testing.T) { + var buf bytes.Buffer + gw := gzip.NewWriter(&buf) + tw := tar.NewWriter(gw) + // Use a directory entry so we don't need to consume a body before returning the error. + err := tw.WriteHeader(&tar.Header{Name: "../../evil", Typeflag: tar.TypeDir, Mode: 0o755}) + require.NoError(t, err) + require.NoError(t, tw.Close()) + require.NoError(t, gw.Close()) + + tmpFile := filepath.Join(t.TempDir(), "bad.tar.gz") + require.NoError(t, os.WriteFile(tmpFile, buf.Bytes(), 0o644)) + targetDir := t.TempDir() + + err = ExtractTarGzFile(tmpFile, targetDir) + require.Error(t, err) + require.Contains(t, err.Error(), "unsafe path") +} + +func TestExtractTarGzFile_symlink_rejected(t *testing.T) { + var buf bytes.Buffer + gw := gzip.NewWriter(&buf) + tw := tar.NewWriter(gw) + err := tw.WriteHeader(&tar.Header{Name: "link", Typeflag: tar.TypeSymlink, Linkname: "/etc/passwd"}) + require.NoError(t, err) + require.NoError(t, tw.Close()) + require.NoError(t, gw.Close()) + + tmpFile := filepath.Join(t.TempDir(), "symlink.tar.gz") + require.NoError(t, os.WriteFile(tmpFile, buf.Bytes(), 0o644)) + targetDir := t.TempDir() + + err = ExtractTarGzFile(tmpFile, targetDir) + require.Error(t, err) + require.Contains(t, err.Error(), "link") +}