diff --git a/pkg/app/app.go b/pkg/app/app.go index db899d9e2..f4cbe876e 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -194,7 +194,7 @@ func (a *App) CurrentAgentSkills() []skills.Skill { // ResolveSkillCommand checks if the input matches a skill slash command (e.g. /skill-name args). // If matched, it reads the skill content and returns the resolved prompt. Otherwise returns "". -func (a *App) ResolveSkillCommand(input string) (string, error) { +func (a *App) ResolveSkillCommand(ctx context.Context, input string) (string, error) { if !strings.HasPrefix(input, "/") { return "", nil } @@ -212,7 +212,7 @@ func (a *App) ResolveSkillCommand(input string) (string, error) { continue } - content, err := st.ReadSkillContent(skill.Name) + content, err := st.ReadSkillContent(ctx, skill.Name) if err != nil { return "", fmt.Errorf("reading skill %q: %w", skill.Name, err) } @@ -229,7 +229,7 @@ func (a *App) ResolveSkillCommand(input string) (string, error) { // ResolveInput resolves the user input by trying skill commands first, // then agent commands. Returns the resolved content ready to send to the agent. func (a *App) ResolveInput(ctx context.Context, input string) string { - if resolved, err := a.ResolveSkillCommand(input); err != nil { + if resolved, err := a.ResolveSkillCommand(ctx, input); err != nil { return fmt.Sprintf("Error loading skill: %v", err) } else if resolved != "" { return resolved diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index 6795847d4..97df36c35 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -245,7 +245,7 @@ func TestApp_ResolveSkillCommand_NoLocalRuntime(t *testing.T) { app := New(ctx, rt, sess) // mockRuntime is not a LocalRuntime, so no skills should be returned - resolved, err := app.ResolveSkillCommand("/some-skill") + resolved, err := app.ResolveSkillCommand(ctx, "/some-skill") require.NoError(t, err) assert.Empty(t, resolved) } @@ -258,7 +258,7 @@ func TestApp_ResolveSkillCommand_NotSlashCommand(t *testing.T) { sess := session.New() app := New(ctx, rt, sess) - resolved, err := app.ResolveSkillCommand("not a slash command") + resolved, err := app.ResolveSkillCommand(ctx, "not a slash command") require.NoError(t, err) assert.Empty(t, resolved) } diff --git a/pkg/skills/expand.go b/pkg/skills/expand.go new file mode 100644 index 000000000..bde2eb83a --- /dev/null +++ b/pkg/skills/expand.go @@ -0,0 +1,85 @@ +package skills + +import ( + "bytes" + "context" + "fmt" + "io" + "log/slog" + "os/exec" + "regexp" + "strings" + "time" +) + +// commandTimeout is the maximum time allowed for a single command expansion. +const commandTimeout = 30 * time.Second + +// maxOutputSize is the maximum number of bytes read from a command's stdout. +const maxOutputSize = 1 << 20 // 1 MB + +// commandPattern matches the !`command` syntax used by Claude Code skills +// to embed dynamic command output into skill content. +var commandPattern = regexp.MustCompile("!`([^`]+)`") + +// ExpandCommands replaces all !`command` patterns in the given content +// with the stdout of executing each command via the system shell. +// Commands are executed with the specified working directory. +// If a command fails, the pattern is replaced with an error message +// rather than failing the entire expansion. +func ExpandCommands(ctx context.Context, content, workDir string) string { + return commandPattern.ReplaceAllStringFunc(content, func(match string) string { + command := match[2 : len(match)-1] // strip leading !` and trailing ` + + output, err := runCommand(ctx, command, workDir) + if err != nil { + slog.Warn("Skill command expansion failed", "command", command, "error", err) + return fmt.Sprintf("[error executing `%s`: %s]", command, err) + } + + return strings.TrimRight(output, "\n") + }) +} + +// runCommand executes a shell command and returns its stdout (up to maxOutputSize bytes). +// The command runs in the specified working directory. +func runCommand(ctx context.Context, command, workDir string) (string, error) { + ctx, cancel := context.WithTimeout(ctx, commandTimeout) + defer cancel() + + cmd := exec.CommandContext(ctx, "sh", "-c", command) + cmd.Dir = workDir + + var stderr bytes.Buffer + cmd.Stderr = &stderr + + stdout, err := cmd.StdoutPipe() + if err != nil { + return "", err + } + + if err := cmd.Start(); err != nil { + return "", err + } + + out, err := io.ReadAll(io.LimitReader(stdout, maxOutputSize)) + if err != nil { + return "", err + } + + // Drain any remaining stdout so the process doesn't block on a full pipe + // and hang until the context timeout kills it. + _, _ = io.Copy(io.Discard, stdout) + + if err := cmd.Wait(); err != nil { + if ctx.Err() == context.DeadlineExceeded { + return "", fmt.Errorf("command timed out after %s", commandTimeout) + } + if stderrMsg := strings.TrimSpace(stderr.String()); stderrMsg != "" { + return "", fmt.Errorf("%w: %s", err, stderrMsg) + } + return "", err + } + + return string(out), nil +} diff --git a/pkg/skills/expand_test.go b/pkg/skills/expand_test.go new file mode 100644 index 000000000..212e421b3 --- /dev/null +++ b/pkg/skills/expand_test.go @@ -0,0 +1,111 @@ +package skills + +import ( + "context" + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func skipOnWindows(t *testing.T) { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("skipping on windows") + } +} + +func TestExpandCommands(t *testing.T) { + skipOnWindows(t) + + tests := []struct { + name string + content string + want string + }{ + { + name: "no patterns", + content: "# My Skill\n\nJust regular markdown content.", + want: "# My Skill\n\nJust regular markdown content.", + }, + { + name: "simple echo", + content: "Hello !`echo world`!", + want: "Hello world!", + }, + { + name: "multiple commands", + content: "Name: !`echo alice`, Age: !`echo 30`", + want: "Name: alice, Age: 30", + }, + { + name: "multiline output", + content: "Files:\n!`printf 'a.go\nb.go\nc.go\n'`\nEnd.", + want: "Files:\na.go\nb.go\nc.go\nEnd.", + }, + { + name: "empty output", + content: "Before !`true` after", + want: "Before after", + }, + { + name: "pipes", + content: "Count: !`printf 'a\nb\nc\n' | wc -l | tr -d ' '`", + want: "Count: 3", + }, + { + name: "preserves regular backticks", + content: "Use `echo hello` to print.\n\nCode: ```go\nfmt.Println()\n```", + want: "Use `echo hello` to print.\n\nCode: ```go\nfmt.Println()\n```", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ExpandCommands(t.Context(), tt.content, t.TempDir()) + assert.Equal(t, tt.want, result) + }) + } +} + +func TestExpandCommands_WorkingDirectory(t *testing.T) { + skipOnWindows(t) + + tmpDir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "test.txt"), []byte("hello"), 0o644)) + + result := ExpandCommands(t.Context(), "Content: !`cat test.txt`", tmpDir) + assert.Equal(t, "Content: hello", result) +} + +func TestExpandCommands_ScriptExecution(t *testing.T) { + skipOnWindows(t) + + tmpDir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "info.sh"), []byte("#!/bin/sh\necho from-script"), 0o755)) + + result := ExpandCommands(t.Context(), "Output: !`./info.sh`", tmpDir) + assert.Equal(t, "Output: from-script", result) +} + +func TestExpandCommands_FailedCommand(t *testing.T) { + skipOnWindows(t) + + result := ExpandCommands(t.Context(), "Before !`nonexistent_command_12345` after", t.TempDir()) + assert.Contains(t, result, "Before ") + assert.Contains(t, result, "[error executing `nonexistent_command_12345`:") + assert.Contains(t, result, " after") +} + +func TestExpandCommands_CancelledContext(t *testing.T) { + skipOnWindows(t) + + ctx, cancel := context.WithCancel(t.Context()) + cancel() + + result := ExpandCommands(ctx, "Result: !`echo hello`", t.TempDir()) + assert.Contains(t, result, "[error executing `echo hello`:") +} diff --git a/pkg/skills/skills.go b/pkg/skills/skills.go index 443b6b21c..0fcd5e782 100644 --- a/pkg/skills/skills.go +++ b/pkg/skills/skills.go @@ -22,6 +22,7 @@ type Skill struct { FilePath string `yaml:"-"` BaseDir string `yaml:"-"` Files []string `yaml:"-"` + Local bool `yaml:"-"` // true for filesystem-loaded skills, false for remote License string `yaml:"license"` Compatibility string `yaml:"compatibility"` Metadata map[string]string `yaml:"metadata"` @@ -308,6 +309,7 @@ func loadSkillFile(path, dirName string) (Skill, bool) { skill.Name = cmp.Or(skill.Name, dirName) skill.FilePath = path skill.BaseDir = filepath.Dir(path) + skill.Local = true return skill, true } diff --git a/pkg/teamloader/teamloader.go b/pkg/teamloader/teamloader.go index 3e9a1a691..2a37e05f7 100644 --- a/pkg/teamloader/teamloader.go +++ b/pkg/teamloader/teamloader.go @@ -218,7 +218,7 @@ func LoadWithConfig(ctx context.Context, agentSource config.Source, runConfig *c if agentConfig.Skills.Enabled() { loadedSkills := skills.Load(agentConfig.Skills.Sources) if len(loadedSkills) > 0 { - agentTools = append(agentTools, builtin.NewSkillsToolset(loadedSkills)) + agentTools = append(agentTools, builtin.NewSkillsToolset(loadedSkills, runConfig.WorkingDir)) } } diff --git a/pkg/tools/builtin/skills.go b/pkg/tools/builtin/skills.go index e802e6fde..38379c00b 100644 --- a/pkg/tools/builtin/skills.go +++ b/pkg/tools/builtin/skills.go @@ -25,12 +25,14 @@ var ( // agent load skill content and supporting resources by name. It hides whether // a skill is local or remote — the agent just sees a name and description. type SkillsToolset struct { - skills []skills.Skill + skills []skills.Skill + workingDir string } -func NewSkillsToolset(loadedSkills []skills.Skill) *SkillsToolset { +func NewSkillsToolset(loadedSkills []skills.Skill, workingDir string) *SkillsToolset { return &SkillsToolset{ - skills: loadedSkills, + skills: loadedSkills, + workingDir: workingDir, } } @@ -49,7 +51,10 @@ func (s *SkillsToolset) findSkill(name string) *skills.Skill { } // ReadSkillContent returns the content of a skill's SKILL.md by name. -func (s *SkillsToolset) ReadSkillContent(name string) (string, error) { +// For local skills, it expands any !`command` patterns in the content by +// executing the commands and replacing the patterns with their stdout output. +// Command expansion is disabled for remote skills to prevent arbitrary code execution. +func (s *SkillsToolset) ReadSkillContent(ctx context.Context, name string) (string, error) { skill := s.findSkill(name) if skill == nil { return "", fmt.Errorf("skill %q not found", name) @@ -60,6 +65,10 @@ func (s *SkillsToolset) ReadSkillContent(name string) (string, error) { return "", err } + if skill.Local { + content = skills.ExpandCommands(ctx, content, s.workingDir) + } + return content, nil } @@ -119,8 +128,8 @@ type readSkillFileArgs struct { Path string `json:"path" jsonschema:"The relative path to the file within the skill (e.g. references/FORMS.md)"` } -func (s *SkillsToolset) handleReadSkill(_ context.Context, args readSkillArgs) (*tools.ToolCallResult, error) { - content, err := s.ReadSkillContent(args.Name) +func (s *SkillsToolset) handleReadSkill(ctx context.Context, args readSkillArgs) (*tools.ToolCallResult, error) { + content, err := s.ReadSkillContent(ctx, args.Name) if err != nil { return tools.ResultError(err.Error()), nil } diff --git a/pkg/tools/builtin/skills_test.go b/pkg/tools/builtin/skills_test.go index ae8b4ab5a..5fc0c20cf 100644 --- a/pkg/tools/builtin/skills_test.go +++ b/pkg/tools/builtin/skills_test.go @@ -3,6 +3,7 @@ package builtin import ( "os" "path/filepath" + "runtime" "testing" "github.com/stretchr/testify/assert" @@ -18,9 +19,9 @@ func TestSkillsToolset_ReadSkillContent_Local(t *testing.T) { st := NewSkillsToolset([]skills.Skill{ {Name: "local-skill", Description: "A local skill", FilePath: skillFile, BaseDir: tmpDir}, - }) + }, "") - content, err := st.ReadSkillContent("local-skill") + content, err := st.ReadSkillContent(t.Context(), "local-skill") require.NoError(t, err) assert.Equal(t, "# Local Skill\nDo the thing.", content) } @@ -28,9 +29,9 @@ func TestSkillsToolset_ReadSkillContent_Local(t *testing.T) { func TestSkillsToolset_ReadSkillContent_NotFound(t *testing.T) { st := NewSkillsToolset([]skills.Skill{ {Name: "exists", Description: "Exists", FilePath: "/tmp/nonexistent"}, - }) + }, "") - _, err := st.ReadSkillContent("does-not-exist") + _, err := st.ReadSkillContent(t.Context(), "does-not-exist") require.Error(t, err) assert.Contains(t, err.Error(), "not found") } @@ -46,7 +47,7 @@ func TestSkillsToolset_ReadSkillFile(t *testing.T) { Name: "my-skill", Description: "My skill", FilePath: filepath.Join(tmpDir, "SKILL.md"), BaseDir: tmpDir, Files: []string{"SKILL.md", "references/FORMS.md"}, }, - }) + }, "") content, err := st.ReadSkillFile("my-skill", "references/FORMS.md") require.NoError(t, err) @@ -59,7 +60,7 @@ func TestSkillsToolset_ReadSkillFile_PathTraversal(t *testing.T) { st := NewSkillsToolset([]skills.Skill{ {Name: "my-skill", Description: "My skill", FilePath: filepath.Join(tmpDir, "SKILL.md"), BaseDir: tmpDir}, - }) + }, "") _, err := st.ReadSkillFile("my-skill", "../../../etc/passwd") require.Error(t, err) @@ -73,7 +74,7 @@ func TestSkillsToolset_ReadSkillFile_PathTraversal(t *testing.T) { func TestSkillsToolset_ReadSkillFile_SkillNotFound(t *testing.T) { st := NewSkillsToolset([]skills.Skill{ {Name: "exists", Description: "Exists", FilePath: "/tmp/test"}, - }) + }, "") _, err := st.ReadSkillFile("nonexistent", "SKILL.md") require.Error(t, err) @@ -84,7 +85,7 @@ func TestSkillsToolset_Instructions(t *testing.T) { st := NewSkillsToolset([]skills.Skill{ {Name: "skill-a", Description: "Does A"}, {Name: "skill-b", Description: "Does B", Files: []string{"SKILL.md", "references/HELP.md"}}, - }) + }, "") instructions := st.Instructions() @@ -103,7 +104,7 @@ func TestSkillsToolset_Instructions(t *testing.T) { func TestSkillsToolset_Instructions_NoFiles(t *testing.T) { st := NewSkillsToolset([]skills.Skill{ {Name: "simple", Description: "Simple skill"}, - }) + }, "") instructions := st.Instructions() @@ -113,17 +114,17 @@ func TestSkillsToolset_Instructions_NoFiles(t *testing.T) { } func TestSkillsToolset_Instructions_Empty(t *testing.T) { - st := NewSkillsToolset(nil) + st := NewSkillsToolset(nil, "") assert.Empty(t, st.Instructions()) - st = NewSkillsToolset([]skills.Skill{}) + st = NewSkillsToolset([]skills.Skill{}, "") assert.Empty(t, st.Instructions()) } func TestSkillsToolset_Tools_WithFiles(t *testing.T) { st := NewSkillsToolset([]skills.Skill{ {Name: "test", Description: "Test skill", Files: []string{"SKILL.md", "references/HELP.md"}}, - }) + }, "") tools, err := st.Tools(t.Context()) require.NoError(t, err) @@ -136,7 +137,7 @@ func TestSkillsToolset_Tools_WithFiles(t *testing.T) { func TestSkillsToolset_Tools_WithoutFiles(t *testing.T) { st := NewSkillsToolset([]skills.Skill{ {Name: "test", Description: "Test skill"}, - }) + }, "") tools, err := st.Tools(t.Context()) require.NoError(t, err) @@ -146,7 +147,7 @@ func TestSkillsToolset_Tools_WithoutFiles(t *testing.T) { } func TestSkillsToolset_Tools_Empty(t *testing.T) { - st := NewSkillsToolset(nil) + st := NewSkillsToolset(nil, "") tools, err := st.Tools(t.Context()) require.NoError(t, err) @@ -158,7 +159,7 @@ func TestSkillsToolset_Skills(t *testing.T) { {Name: "a", Description: "A"}, {Name: "b", Description: "B"}, } - st := NewSkillsToolset(input) + st := NewSkillsToolset(input, "") assert.Equal(t, input, st.Skills()) } @@ -170,7 +171,7 @@ func TestSkillsToolset_HandleReadSkill(t *testing.T) { st := NewSkillsToolset([]skills.Skill{ {Name: "test-skill", Description: "Test", FilePath: skillFile, BaseDir: tmpDir}, - }) + }, "") result, err := st.handleReadSkill(t.Context(), readSkillArgs{Name: "test-skill"}) require.NoError(t, err) @@ -181,7 +182,7 @@ func TestSkillsToolset_HandleReadSkill(t *testing.T) { func TestSkillsToolset_HandleReadSkill_NotFound(t *testing.T) { st := NewSkillsToolset([]skills.Skill{ {Name: "exists", Description: "Exists", FilePath: "/tmp/test"}, - }) + }, "") result, err := st.handleReadSkill(t.Context(), readSkillArgs{Name: "missing"}) require.NoError(t, err) @@ -200,7 +201,7 @@ func TestSkillsToolset_HandleReadSkillFile(t *testing.T) { Name: "my-skill", Description: "My skill", FilePath: filepath.Join(tmpDir, "SKILL.md"), BaseDir: tmpDir, Files: []string{"SKILL.md", "scripts/deploy.sh"}, }, - }) + }, "") result, err := st.handleReadSkillFile(t.Context(), readSkillFileArgs{SkillName: "my-skill", Path: "scripts/deploy.sh"}) require.NoError(t, err) @@ -214,10 +215,68 @@ func TestSkillsToolset_HandleReadSkillFile_PathTraversal(t *testing.T) { st := NewSkillsToolset([]skills.Skill{ {Name: "my-skill", Description: "My skill", FilePath: filepath.Join(tmpDir, "SKILL.md"), BaseDir: tmpDir}, - }) + }, "") result, err := st.handleReadSkillFile(t.Context(), readSkillFileArgs{SkillName: "my-skill", Path: "../../../etc/passwd"}) require.NoError(t, err) assert.True(t, result.IsError) assert.Contains(t, result.Output, "invalid file path") } + +func TestSkillsToolset_ReadSkillContent_ExpandsCommands(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("skipping on windows") + } + + tmpDir := t.TempDir() + skillFile := filepath.Join(tmpDir, "SKILL.md") + content := "# Skill\nBranch: !`echo main`\nDone." + require.NoError(t, os.WriteFile(skillFile, []byte(content), 0o644)) + + st := NewSkillsToolset([]skills.Skill{ + {Name: "expand-skill", Description: "Expands commands", FilePath: skillFile, BaseDir: tmpDir, Local: true}, + }, tmpDir) + + result, err := st.ReadSkillContent(t.Context(), "expand-skill") + require.NoError(t, err) + assert.Equal(t, "# Skill\nBranch: main\nDone.", result) +} + +func TestSkillsToolset_ReadSkillContent_ExpandsScript(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("skipping on windows") + } + + tmpDir := t.TempDir() + + // Create a script in the working directory + scriptPath := filepath.Join(tmpDir, "gather.sh") + require.NoError(t, os.WriteFile(scriptPath, []byte("#!/bin/sh\necho gathered-data"), 0o755)) + + skillFile := filepath.Join(tmpDir, "SKILL.md") + content := "Data: !`./gather.sh`" + require.NoError(t, os.WriteFile(skillFile, []byte(content), 0o644)) + + st := NewSkillsToolset([]skills.Skill{ + {Name: "script-skill", Description: "Runs scripts", FilePath: skillFile, BaseDir: tmpDir, Local: true}, + }, tmpDir) + + result, err := st.ReadSkillContent(t.Context(), "script-skill") + require.NoError(t, err) + assert.Equal(t, "Data: gathered-data", result) +} + +func TestSkillsToolset_ReadSkillContent_RemoteSkillSkipsExpansion(t *testing.T) { + tmpDir := t.TempDir() + skillFile := filepath.Join(tmpDir, "SKILL.md") + content := "Info: !`echo should-not-run`" + require.NoError(t, os.WriteFile(skillFile, []byte(content), 0o644)) + + st := NewSkillsToolset([]skills.Skill{ + {Name: "remote-skill", Description: "Remote", FilePath: skillFile, BaseDir: tmpDir, Local: false}, + }, "") + + result, err := st.ReadSkillContent(t.Context(), "remote-skill") + require.NoError(t, err) + assert.Equal(t, content, result, "commands in remote skills must not be expanded") +}