Files
zero/internal/tools/skill_test.go
363c0312d6 Skills & plugins: skill tool, skills loader, plugin manifests (#123)
* Skills & plugins: skill tool, skills loader, plugin manifests

Module of the runtime-core split (off main, after #119 tools merged). Clean extract — internal/skills is new; internal/plugins had no drift on main. No new deps.

- internal/skills: loads */SKILL.md frontmatter from the skills dir (resolves its own XDG dir); no YAML dependency.

- skill tool (internal/tools/skill.go), read-only, registered in CoreReadOnlyTools (so it's in the agent core + the MCP read-only default). Path-safe: it Load()s all skills and matches by exact name — it never builds a path from the model-supplied arg, so there is no traversal surface. knownToolNames gains "skill" for the specialist cross-package invariant; CoreReadOnlyTools count test bumped 4->5.

- internal/plugins: plugin manifest metadata enrichment (Author/License/Keywords/Interface). FORWARD code — the cli plugin-loading wiring lands later.

MCP hardening (nested-schema + hung-server fix) is a SEPARATE follow-up PR. build/vet/-race/full-suite + GOOS=windows build green.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

* Address #123 review (CodeRabbit): pointer metadata, skill schema alias, dir fallback, List test

- plugins: Author/Interface are now *PluginAuthor/*PluginInterface so omitempty actually omits them — a non-pointer struct is never empty to encoding/json, so the old form emitted author:{}/interface:{} and changed the serialized shape of plugins that don't set them. parseAuthor/parseInterface return nil when all fields are empty; formatAuthor takes the pointer. (Major)

- skill: schema now declares the 'skill' alias alongside 'name' (and drops strict Required) so the alias survives schema validators that reject unknown keys (AdditionalProperties:false); Run still enforces exactly-one-of via aliasedStringArg. (Major)

- skills.DefaultDir: return "" instead of a relative ".local/share/..." path when neither XDG_DATA_HOME nor a resolvable HOME exists, so skills can't bind to the process CWD (load("") already no-ops). (Minor)

- skills_test: TestListReturnsNamesAndDescriptions now asserts List strips Content so a listing can't silently leak full skill bodies. (Minor)

build/vet/-race/full-suite + GOOS=windows build green.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

* Address #123 review (@Vasanthdev2004): confine skill SKILL.md to the skills root

skill is a permission-allow read-only core/MCP tool, so the loader must not follow a symlinked SKILL.md (or skill dir) out of the skills root and become an arbitrary-file reader. load() now resolves the skills root via EvalSymlinks and, for each entry, resolves SKILL.md through symlinks and verifies the real path stays under the root (confineSkillPath); an escaping or unreadable path is skipped rather than read. Mirrors the grep/read_file confinement. Legit skills under a symlinked root (e.g. macOS /tmp) still load because the root is resolved too.

Regression: TestLoadSkipsSymlinkedSkillFileEscapingRoot (a SKILL.md symlinked to a secret outside the root is skipped, not read).

build/vet/-race/full-suite + GOOS=windows build green.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

* Address #123 re-review (CodeRabbit): reject non-regular SKILL.md, single-trim metadata

- skills: confineSkillPath now Lstats the resolved path and skips non-regular targets (directory/FIFO/device/socket). A FIFO/device named SKILL.md (or an in-root symlink to one) would otherwise make os.ReadFile block indefinitely — skill is a permission-allow tool over a user-controlled dir. Regression: TestLoadSkipsNonRegularSkillFile (unix-tagged; mkfifo SKILL.md, asserts Load skips it within a timeout rather than hanging). (Major)

- plugins: coerceMetaStringSlice trims each item once into a local var instead of calling strings.TrimSpace twice. (Nitpick)

build/vet/-race/full-suite + GOOS=windows build green.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

---------

Co-authored-by: KRATOS <[email protected]>
Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
2026-06-07 21:35:11 +05:30

99 lines
3.1 KiB
Go

package tools
import (
"context"
"os"
"path/filepath"
"strings"
"testing"
)
func writeSkillFile(t *testing.T, dir string, name string, content string) {
t.Helper()
skillDir := filepath.Join(dir, name)
if err := os.MkdirAll(skillDir, 0o755); err != nil {
t.Fatalf("mkdir %s: %v", skillDir, err)
}
if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(content), 0o644); err != nil {
t.Fatalf("write SKILL.md: %v", err)
}
}
func TestSkillToolIsReadOnly(t *testing.T) {
tool := NewSkillTool(t.TempDir())
if tool.Name() != "skill" {
t.Fatalf("Name = %q, want skill", tool.Name())
}
if tool.Safety().SideEffect != SideEffectRead {
t.Fatalf("SideEffect = %s, want read", tool.Safety().SideEffect)
}
if tool.Safety().Permission != PermissionAllow {
t.Fatalf("Permission = %s, want allow", tool.Safety().Permission)
}
if tool.Parameters().Type != "object" {
t.Fatalf("schema type = %s, want object", tool.Parameters().Type)
}
}
func TestSkillToolReturnsContentForKnownSkill(t *testing.T) {
dir := t.TempDir()
writeSkillFile(t, dir, "confirmation-policy", "---\nname: confirmation-policy\ndescription: ask first\n---\n\n# Confirmation Policy\n\nAsk before risky actions.")
tool := NewSkillTool(dir)
result := tool.Run(context.Background(), map[string]any{"name": "confirmation-policy"})
if result.Status != StatusOK {
t.Fatalf("Status = %s, want ok (output: %s)", result.Status, result.Output)
}
if !strings.Contains(result.Output, "Ask before risky actions.") {
t.Fatalf("Output missing skill body: %q", result.Output)
}
}
func TestSkillToolAcceptsSkillAlias(t *testing.T) {
dir := t.TempDir()
writeSkillFile(t, dir, "demo", "body of demo")
tool := NewSkillTool(dir)
result := tool.Run(context.Background(), map[string]any{"skill": "demo"})
if result.Status != StatusOK {
t.Fatalf("Status = %s, want ok (output: %s)", result.Status, result.Output)
}
if !strings.Contains(result.Output, "body of demo") {
t.Fatalf("Output = %q", result.Output)
}
}
func TestSkillToolUnknownSkillErrorsAndListsAvailable(t *testing.T) {
dir := t.TempDir()
writeSkillFile(t, dir, "alpha", "a")
writeSkillFile(t, dir, "beta", "b")
tool := NewSkillTool(dir)
result := tool.Run(context.Background(), map[string]any{"name": "missing"})
if result.Status != StatusError {
t.Fatalf("Status = %s, want error", result.Status)
}
if !strings.Contains(result.Output, "alpha") || !strings.Contains(result.Output, "beta") {
t.Fatalf("error should list available skills, got: %q", result.Output)
}
}
func TestSkillToolMissingNameErrors(t *testing.T) {
tool := NewSkillTool(t.TempDir())
result := tool.Run(context.Background(), map[string]any{})
if result.Status != StatusError {
t.Fatalf("Status = %s, want error", result.Status)
}
}
func TestSkillToolNoSkillsAvailable(t *testing.T) {
tool := NewSkillTool(filepath.Join(t.TempDir(), "missing"))
result := tool.Run(context.Background(), map[string]any{"name": "anything"})
if result.Status != StatusError {
t.Fatalf("Status = %s, want error", result.Status)
}
if !strings.Contains(strings.ToLower(result.Output), "no skills") {
t.Fatalf("expected a no-skills message, got: %q", result.Output)
}
}