From 64fb7488975f1aa4c8c4def7e25bfa20731fafa6 Mon Sep 17 00:00:00 2001 From: Greg Gauthier Date: Sat, 7 Mar 2026 17:34:59 +0000 Subject: [PATCH] refactor(recipe): simplify path resolution and remove shell command execution - Consolidate resolveWorkDir and remove resolvePackagePath for cleaner path handling. - Eliminate executeShellCommands and related logic to disable shell execution in recipes. - Simplify error messaging in loader for unsafe commands. --- internal/recipe/loader.go | 2 +- internal/recipe/runner.go | 152 +------------------------------------- 2 files changed, 4 insertions(+), 150 deletions(-) diff --git a/internal/recipe/loader.go b/internal/recipe/loader.go index 5a890ec..bb42651 100644 --- a/internal/recipe/loader.go +++ b/internal/recipe/loader.go @@ -51,7 +51,7 @@ func Load(path string, userParams map[string]any) (*Recipe, error) { for _, cmd := range r.AllowedShellCommands { trimmed := strings.TrimSpace(strings.ToLower(cmd)) if !safeReadOnlyCommands[trimmed] && !strings.HasPrefix(trimmed, "git status") && !strings.HasPrefix(trimmed, "git log") { - return nil, fmt.Errorf("\033[31mERROR: Recipe contains unsafe shell command: %q\033[0m\n\nOnly the following read-only commands are allowed:\n ls, pwd, cat, tree, git status, git log, find, grep\n\nRemove or replace the dangerous command and try again.", cmd) + return nil, fmt.Errorf("\u001B[31mERROR: Recipe contains unsafe shell command: %q\u001B[0m Remove or replace the dangerous command and try again", cmd) } } diff --git a/internal/recipe/runner.go b/internal/recipe/runner.go index 48aac91..182a96d 100644 --- a/internal/recipe/runner.go +++ b/internal/recipe/runner.go @@ -4,7 +4,6 @@ import ( "encoding/json" "fmt" "os" - "os/exec" "path/filepath" "strings" @@ -24,7 +23,6 @@ func NewRunner(r *Recipe, client *grok.Client, model string) *Runner { func (r *Runner) Run() error { fmt.Printf("🍳 Starting recipe: %s v%s\n\n", r.Recipe.Name, r.Recipe.Version) - // Resolve the final working directory once (package_path + project_name) workDir := r.resolveWorkDir() var previousResults []string @@ -50,13 +48,6 @@ func (r *Runner) Run() error { r.handleApplyStep(refactorJSONs) continue - case strings.Contains(titleLower, "initialize") || strings.Contains(titleLower, "create") || - strings.Contains(titleLower, "run") || strings.Contains(titleLower, "shell") || - strings.Contains(titleLower, "poetry") || strings.Contains(titleLower, "git") || - strings.Contains(titleLower, "tea"): - r.executeShellCommands(step, previousResults, workDir) - continue - default: prompt := fmt.Sprintf(`Recipe Overview: %s @@ -92,9 +83,8 @@ Execute this step now. Respond ONLY with the expected output format — no expla return nil } -// resolveWorkDir returns the absolute path where all shell commands should run +// resolveWorkDir expands ~ and makes absolute (keeps your boundary logic) func (r *Runner) resolveWorkDir() string { - // Start with package_path param or default root := "." if v, ok := r.Recipe.ResolvedParams["package_path"]; ok { if s, ok := v.(string); ok && s != "" { @@ -102,7 +92,6 @@ func (r *Runner) resolveWorkDir() string { } } - // Expand ~ if strings.HasPrefix(root, "~/") { home, _ := os.UserHomeDir() root = filepath.Join(home, root[2:]) @@ -110,49 +99,11 @@ func (r *Runner) resolveWorkDir() string { root, _ = os.UserHomeDir() } - // Make absolute - absRoot, err := filepath.Abs(root) - if err != nil { - absRoot = root - } - - // Append project_name if provided - if name, ok := r.Recipe.ResolvedParams["project_name"]; ok { - if s, ok := name.(string); ok && s != "" { - absRoot = filepath.Join(absRoot, s) - } - } - - return absRoot -} - -// resolvePackagePath expands ~, ., and makes the path absolute -func (r *Runner) resolvePackagePath() string { - root := "." - if v, ok := r.Recipe.ResolvedParams["package_path"]; ok { - if s, ok := v.(string); ok && s != "" { - root = s - } - } - - // Expand ~ - if strings.HasPrefix(root, "~/") { - home, _ := os.UserHomeDir() - root = filepath.Join(home, root[2:]) - } else if root == "~" { - root, _ = os.UserHomeDir() - } - - // Make absolute - abs, err := filepath.Abs(root) - if err != nil { - abs = root - } + abs, _ := filepath.Abs(root) return abs } -// discoverFiles now takes the resolved absolute path -// discoverFiles now takes the resolved workDir +// discoverFiles — uses the resolved workDir (your current signature) func (r *Runner) discoverFiles(workDir string) []string { var files []string @@ -298,100 +249,3 @@ func createUnifiedPatch(changes []FileChange, patchPath string) error { } return nil } - -// executeShellCommands — safe, whitelisted, boundary-enforced -// executeShellCommands — safe, whitelisted, boundary-enforced -func (r *Runner) executeShellCommands(step Step, previousResults []string, workDir string) { - prompt := fmt.Sprintf(`You are executing shell commands for this step. - -Project root (current working directory for all commands): %s - -Previous step results: -%s - -=== CURRENT STEP === -Objective: %s -Instructions: %s - -Return ONLY a JSON array of commands. Use **relative paths only** (no ~, no absolute paths). Example: - -[ - { - "command": "poetry", - "args": ["new", "."] - } -] - -Never use cd, rm -rf, or anything that could escape the project directory.`, - workDir, - strings.Join(previousResults, "\n\n---\n\n"), - step.Objective, - step.Instructions) - - messages := []map[string]string{ - {"role": "system", "content": "You are Grok, built by xAI. Precise expert programmer and refactoring assistant."}, - {"role": "user", "content": prompt}, - } - - response := r.Client.Stream(messages, r.Model) - fmt.Println() - - type ShellCommand struct { - Command string `json:"command"` - Args []string `json:"args"` - } - - var cmds []ShellCommand - start := strings.Index(response, "[") - end := strings.LastIndex(response, "]") + 1 - if start == -1 { - fmt.Println(" ⚠️ No valid shell commands returned — skipping.") - return - } - - if err := json.Unmarshal([]byte(response[start:end]), &cmds); err != nil { - fmt.Printf(" ⚠️ Could not parse shell commands: %v\n", err) - return - } - - for _, cmd := range cmds { - fullCmd := cmd.Command - if len(cmd.Args) > 0 { - fullCmd += " " + strings.Join(cmd.Args, " ") - } - - fmt.Printf(" Running: %s\n", fullCmd) - - // Whitelist check - allowed := false - for _, allowedCmd := range r.Recipe.AllowedShellCommands { - if strings.HasPrefix(cmd.Command, allowedCmd) { - allowed = true - break - } - } - if !allowed { - fmt.Printf(" ❌ Command not allowed: %s\n", cmd.Command) - continue - } - - // Safety check: no escaping the boundary - for _, arg := range cmd.Args { - if strings.Contains(arg, "..") || strings.HasPrefix(arg, "/") { - fmt.Printf(" ❌ Command rejected (tries to escape boundary): %s\n", fullCmd) - continue - } - } - - // Execute with strict cwd - execCmd := exec.Command(cmd.Command, cmd.Args...) - execCmd.Dir = workDir - output, err := execCmd.CombinedOutput() - - if err != nil { - fmt.Printf(" ❌ Failed: %v\n%s\n", err, string(output)) - } else { - fmt.Printf(" ✅ Success\n%s\n", string(output)) - } - } -}