diff --git a/cmd/update/update_test.go b/cmd/update/update_test.go index 5cfe52477..ab3382812 100644 --- a/cmd/update/update_test.go +++ b/cmd/update/update_test.go @@ -9,6 +9,8 @@ import ( "encoding/json" "errors" "fmt" + "net/http" + "net/http/httptest" "os/exec" "strings" "testing" @@ -28,6 +30,22 @@ func newTestFactory(t *testing.T) (*cmdutil.Factory, *bytes.Buffer, *bytes.Buffe return f, stdout, stderr } +func officialSkillsIndexURLForTest(t *testing.T) string { + t.Helper() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + t.Fatalf("method = %s, want GET", r.Method) + } + if r.URL.Path != "/.well-known/skills/index.json" { + t.Fatalf("path = %s, want /.well-known/skills/index.json", r.URL.Path) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"skills":[{"name":"lark-calendar"},{"name":"lark-mail"}]}`)) + })) + t.Cleanup(server.Close) + return server.URL + "/.well-known/skills/index.json" +} + // mockDetect sets up newUpdater to return an Updater with the given DetectResult. func mockDetect(t *testing.T, result selfupdate.DetectResult) { t.Helper() @@ -35,6 +53,8 @@ func mockDetect(t *testing.T, result selfupdate.DetectResult) { newUpdater = func() *selfupdate.Updater { u := selfupdate.New() u.DetectOverride = func() selfupdate.DetectResult { return result } + u.OfficialSkillsIndexURL = officialSkillsIndexURLForTest(t) + u.SkillsCommandOverride = successfulSkillsCommand() return u } t.Cleanup(func() { newUpdater = origNew }) @@ -49,6 +69,7 @@ func mockDetectAndNpm(t *testing.T, result selfupdate.DetectResult, npmFn func(s u.DetectOverride = func() selfupdate.DetectResult { return result } u.NpmInstallOverride = npmFn u.VerifyOverride = func(string) error { return nil } + u.OfficialSkillsIndexURL = officialSkillsIndexURLForTest(t) u.SkillsCommandOverride = successfulSkillsCommand() return u } @@ -59,10 +80,14 @@ func successfulSkillsCommand() func(args ...string) *selfupdate.NpmResult { return func(args ...string) *selfupdate.NpmResult { r := &selfupdate.NpmResult{} switch strings.Join(args, " ") { - case "-y skills add https://open.feishu.cn --list": - r.Stdout.WriteString("Available Skills\n │ lark-calendar\n │ lark-mail\n") case "-y skills ls -g": r.Stdout.WriteString("Global Skills\nlark-calendar /tmp/lark-calendar\ncustom-skill /tmp/custom-skill\n") + case "-y skills add https://open.feishu.cn -s lark-calendar lark-mail -g -y": + // incremental install succeeds + case "-y skills add https://open.feishu.cn -g -y": + // full install succeeds + case "-y skills add larksuite/cli -g -y": + // fallback full install succeeds default: } return r diff --git a/internal/selfupdate/updater.go b/internal/selfupdate/updater.go index 8cba0b7d3..56777c673 100644 --- a/internal/selfupdate/updater.go +++ b/internal/selfupdate/updater.go @@ -10,7 +10,10 @@ import ( "bytes" "context" "fmt" + "io" + "net/http" "os/exec" + "regexp" "strings" "time" @@ -24,6 +27,8 @@ import ( // Tests that mutate execLookPath must not call t.Parallel(). var execLookPath = exec.LookPath +var officialSkillNamePattern = regexp.MustCompile(`^lark-[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$`) + // InstallMethod describes how the CLI was installed. type InstallMethod int @@ -42,6 +47,13 @@ const ( verifyTimeout = 10 * time.Second ) +const ( + officialSkillsIndexDefaultURL = "https://open.feishu.cn/.well-known/skills/index.json" + officialSkillsIndexMaxBytes = 2 << 20 +) + +var officialSkillsIndexURL = officialSkillsIndexDefaultURL + // DetectResult holds installation detection results. type DetectResult struct { Method InstallMethod @@ -86,6 +98,7 @@ type Updater struct { SkillsCommandOverride func(args ...string) *NpmResult VerifyOverride func(expectedVersion string) error RestoreAvailableOverride func() bool + OfficialSkillsIndexURL string // backupCreated is set to true by PrepareSelfReplace (Windows) when the // running binary is successfully renamed to .old. Used by @@ -154,10 +167,53 @@ func (u *Updater) RunNpmInstall(version string) *NpmResult { } func (u *Updater) ListOfficialSkills() *NpmResult { - r := u.runSkillsListOfficial("https://open.feishu.cn") - if r.Err != nil { - r = u.runSkillsListOfficial("larksuite/cli") + return u.fetchOfficialSkillsIndex() +} + +func (u *Updater) fetchOfficialSkillsIndex() *NpmResult { + r := &NpmResult{} + indexURL := officialSkillsIndexURL + if u.OfficialSkillsIndexURL != "" { + indexURL = u.OfficialSkillsIndexURL + } + + ctx, cancel := context.WithTimeout(context.Background(), skillsUpdateTimeout) + defer cancel() + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, indexURL, nil) + if err != nil { + r.Err = fmt.Errorf("create official skills index request %s: %w", indexURL, err) + return r + } + req.Header.Set("Accept", "application/json") + + resp, err := http.DefaultClient.Do(req) + if err != nil { + if ctx.Err() == context.DeadlineExceeded { + r.Err = fmt.Errorf("official skills index fetch timed out after %s: %s", skillsUpdateTimeout, indexURL) + return r + } + r.Err = fmt.Errorf("fetch official skills index %s: %w", indexURL, err) + return r + } + defer resp.Body.Close() + + if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { + r.Err = fmt.Errorf("fetch official skills index %s: HTTP %s", indexURL, resp.Status) + return r + } + + body, err := io.ReadAll(io.LimitReader(resp.Body, officialSkillsIndexMaxBytes+1)) + if err != nil { + r.Err = fmt.Errorf("read official skills index %s: %w", indexURL, err) + return r + } + if len(body) > officialSkillsIndexMaxBytes { + r.Err = fmt.Errorf("official skills index %s exceeds %d bytes", indexURL, officialSkillsIndexMaxBytes) + return r } + + _, _ = r.Stdout.Write(body) return r } @@ -166,6 +222,10 @@ func (u *Updater) ListGlobalSkills() *NpmResult { } func (u *Updater) InstallSkill(nameList []string) *NpmResult { + if err := validateOfficialSkillNames(nameList); err != nil { + return &NpmResult{Err: err} + } + r := u.runSkillsInstall("https://open.feishu.cn", nameList) if r.Err != nil { r = u.runSkillsInstall("larksuite/cli", nameList) @@ -173,6 +233,15 @@ func (u *Updater) InstallSkill(nameList []string) *NpmResult { return r } +func validateOfficialSkillNames(nameList []string) error { + for _, name := range nameList { + if !officialSkillNamePattern.MatchString(name) { + return fmt.Errorf("invalid official skill name %q", name) + } + } + return nil +} + func (u *Updater) InstallAllSkills() *NpmResult { r := u.runSkillsAdd("https://open.feishu.cn") if r.Err != nil { @@ -185,10 +254,6 @@ func (u *Updater) runSkillsAdd(source string) *NpmResult { return u.runSkillsCommand("-y", "skills", "add", source, "-g", "-y") } -func (u *Updater) runSkillsListOfficial(source string) *NpmResult { - return u.runSkillsCommand("-y", "skills", "add", source, "--list") -} - func (u *Updater) runSkillsListGlobal() *NpmResult { return u.runSkillsCommand("-y", "skills", "ls", "-g") } diff --git a/internal/selfupdate/updater_test.go b/internal/selfupdate/updater_test.go index 458f3f79b..24999ba5e 100644 --- a/internal/selfupdate/updater_test.go +++ b/internal/selfupdate/updater_test.go @@ -5,6 +5,8 @@ package selfupdate import ( "fmt" + "net/http" + "net/http/httptest" "os" "path/filepath" "runtime" @@ -174,13 +176,6 @@ func TestSkillsCommandsUseExpectedArgs(t *testing.T) { run func(*Updater) *NpmResult want string }{ - { - name: "list official primary", - run: func(u *Updater) *NpmResult { - return u.runSkillsListOfficial("https://open.feishu.cn") - }, - want: "-y skills add https://open.feishu.cn --list", - }, { name: "list global", run: func(u *Updater) *NpmResult { @@ -225,29 +220,90 @@ func TestSkillsCommandsUseExpectedArgs(t *testing.T) { } } -func TestListOfficialSkillsFallsBack(t *testing.T) { - called := []string{} - updater := &Updater{ +func TestListOfficialSkillsFetchesIndexJSON(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + t.Fatalf("method = %s, want GET", r.Method) + } + if r.URL.Path != "/.well-known/skills/index.json" { + t.Fatalf("path = %s, want /.well-known/skills/index.json", r.URL.Path) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"skills":[{"name":"lark-calendar"}]}`)) + })) + defer server.Close() + + oldURL := officialSkillsIndexURL + officialSkillsIndexURL = server.URL + "/.well-known/skills/index.json" + defer func() { officialSkillsIndexURL = oldURL }() + + u := New() + result := u.ListOfficialSkills() + if result.Err != nil { + t.Fatalf("ListOfficialSkills() err = %v, want nil", result.Err) + } + if got := result.Stdout.String(); !strings.Contains(got, `"lark-calendar"`) { + t.Fatalf("ListOfficialSkills() stdout = %q, want index JSON", got) + } +} + +func TestListOfficialSkillsNon2xxFails(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "nope", http.StatusBadGateway) + })) + defer server.Close() + + oldURL := officialSkillsIndexURL + officialSkillsIndexURL = server.URL + "/.well-known/skills/index.json" + defer func() { officialSkillsIndexURL = oldURL }() + + u := New() + result := u.ListOfficialSkills() + if result.Err == nil { + t.Fatal("ListOfficialSkills() err = nil, want error") + } + if !strings.Contains(result.Err.Error(), "502") { + t.Fatalf("ListOfficialSkills() err = %v, want HTTP status", result.Err) + } +} + +func TestListOfficialSkillsTooLargeFails(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(strings.Repeat("x", officialSkillsIndexMaxBytes+1))) + })) + defer server.Close() + + oldURL := officialSkillsIndexURL + officialSkillsIndexURL = server.URL + "/.well-known/skills/index.json" + defer func() { officialSkillsIndexURL = oldURL }() + + u := New() + result := u.ListOfficialSkills() + if result.Err == nil { + t.Fatal("ListOfficialSkills() err = nil, want error") + } + if !strings.Contains(result.Err.Error(), "exceeds") { + t.Fatalf("ListOfficialSkills() err = %v, want size limit error", result.Err) + } +} + +func TestInstallSkillRejectsInvalidOfficialNames(t *testing.T) { + called := false + u := &Updater{ SkillsCommandOverride: func(args ...string) *NpmResult { - called = append(called, strings.Join(args, " ")) - r := &NpmResult{} - if strings.Contains(strings.Join(args, " "), "https://open.feishu.cn") { - r.Err = fmt.Errorf("primary failed") - return r - } - r.Stdout.WriteString("lark-calendar\n") - return r + called = true + return &NpmResult{} }, } - result := updater.ListOfficialSkills() - if result.Err != nil { - t.Fatalf("ListOfficialSkills() err = %v, want nil", result.Err) + result := u.InstallSkill([]string{"lark-calendar", "lark-calendar@evil"}) + if result.Err == nil { + t.Fatal("InstallSkill() err = nil, want invalid name error") } - if len(called) != 2 { - t.Fatalf("called %d commands, want 2: %#v", len(called), called) + if !strings.Contains(result.Err.Error(), "invalid official skill name") { + t.Fatalf("InstallSkill() err = %v, want invalid name error", result.Err) } - if !strings.Contains(called[1], "larksuite/cli --list") { - t.Fatalf("fallback call = %q, want larksuite/cli --list", called[1]) + if called { + t.Fatal("InstallSkill() called skills command for invalid name") } } diff --git a/internal/skillscheck/sync.go b/internal/skillscheck/sync.go index f2d2a2cf8..d025f4798 100644 --- a/internal/skillscheck/sync.go +++ b/internal/skillscheck/sync.go @@ -4,6 +4,7 @@ package skillscheck import ( + "encoding/json" "fmt" "regexp" "sort" @@ -14,8 +15,9 @@ import ( ) var ( - skillNamePattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9_:-]*(@[^\s]+)?$`) - ansiPattern = regexp.MustCompile(`\x1b\[[0-?]*[ -/]*[@-~]`) + skillNamePattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9_:-]*(@[^\s]+)?$`) + officialSkillNamePattern = regexp.MustCompile(`^lark-[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$`) + ansiPattern = regexp.MustCompile(`\x1b\[[0-?]*[ -/]*[@-~]`) ) type SyncInput struct { @@ -41,6 +43,10 @@ func stripANSI(s string) string { func ParseSkillsList(text string) []string { text = stripANSI(text) + if skills := parseOfficialSkillsIndex(text); skills != nil { + return skills + } + lines := strings.Split(text, "\n") // Detect format type @@ -57,6 +63,40 @@ func ParseSkillsList(text string) []string { return nil } +type officialSkillsIndex struct { + Skills []map[string]interface{} `json:"skills"` +} + +func parseOfficialSkillsIndex(text string) []string { + trimmed := strings.TrimSpace(text) + if trimmed == "" || !strings.HasPrefix(trimmed, "{") { + return nil + } + + var index officialSkillsIndex + if err := json.Unmarshal([]byte(trimmed), &index); err != nil { + return nil + } + if index.Skills == nil { + return nil + } + + seen := map[string]bool{} + for _, skill := range index.Skills { + name, ok := skill["name"].(string) + if !ok { + continue + } + name = strings.TrimSpace(name) + if !officialSkillNamePattern.MatchString(name) { + continue + } + seen[name] = true + } + + return sortedKeys(seen) +} + // parseGlobalSkillsList parses the output of "npx -y skills ls -g" func parseGlobalSkillsList(lines []string) []string { seen := map[string]bool{} diff --git a/internal/skillscheck/sync_test.go b/internal/skillscheck/sync_test.go index 18a2802c6..0a2e3f343 100644 --- a/internal/skillscheck/sync_test.go +++ b/internal/skillscheck/sync_test.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "reflect" + "strconv" "strings" "testing" "time" @@ -67,6 +68,56 @@ func TestParseGlobalSkillsListWithANSI(t *testing.T) { } } +func TestParseSkillsListOfficialJSONIndex(t *testing.T) { + input := `{ + "skills": [ + {"name": "lark-mail", "description": "Mail", "files": ["SKILL.md"]}, + {"name": "lark-calendar", "description": "Calendar", "files": ["SKILL.md"]}, + {"name": "lark-mail", "description": "Duplicate", "files": ["SKILL.md"]} + ] + }` + + got := ParseSkillsList(input) + want := []string{"lark-calendar", "lark-mail"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("ParseSkillsList() JSON index = %#v, want %#v", got, want) + } +} + +func TestParseSkillsListOfficialJSONIndexIgnoresInvalidNames(t *testing.T) { + input := `{ + "skills": [ + {"name": " lark-calendar "}, + {"name": "custom-skill"}, + {"name": "lark-bad name"}, + {"name": "lark-../../evil"}, + {"name": "lark-calendar@evil"}, + {"name": "lark-calendar:evil"}, + {"name": "lark-calendar/evil"}, + {"name": "lark-calendar..evil"}, + {"name": "lark-calendar_evil"}, + {"name": ""}, + {"description": "missing name"}, + {"name": 42} + ] + }` + + got := ParseSkillsList(input) + want := []string{"lark-calendar"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("ParseSkillsList() JSON index invalid names = %#v, want %#v", got, want) + } +} + +func TestParseOfficialSkillsListLegacyAvailableSkills(t *testing.T) { + input := "Available Skills\n │ lark-calendar\n │ lark-mail\n" + got := ParseSkillsList(input) + want := []string{"lark-calendar", "lark-mail"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("ParseSkillsList() legacy Available Skills = %#v, want %#v", got, want) + } +} + func TestPlanNormal_WithReadableStatePreservesDeletedAndAddsNew(t *testing.T) { previous := &SkillsState{OfficialSkills: []string{"lark-calendar", "lark-mail"}} got := PlanSync(SyncInput{ @@ -125,12 +176,16 @@ type fakeSkillsRunner struct { func officialSkillsOutput(names ...string) string { var b strings.Builder - b.WriteString("Available Skills\n") - for _, name := range names { - b.WriteString("│ ") - b.WriteString(name) - b.WriteString("\n") - } + b.WriteString(`{"skills":[`) + for i, name := range names { + if i > 0 { + b.WriteString(",") + } + b.WriteString(`{"name":`) + b.WriteString(strconv.Quote(name)) + b.WriteString(`,"description":"","files":["SKILL.md"]}`) + } + b.WriteString(`]}`) return b.String() }