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.
This commit is contained in:
parent
63e640c022
commit
64fb748897
@ -51,7 +51,7 @@ func Load(path string, userParams map[string]any) (*Recipe, error) {
|
|||||||
for _, cmd := range r.AllowedShellCommands {
|
for _, cmd := range r.AllowedShellCommands {
|
||||||
trimmed := strings.TrimSpace(strings.ToLower(cmd))
|
trimmed := strings.TrimSpace(strings.ToLower(cmd))
|
||||||
if !safeReadOnlyCommands[trimmed] && !strings.HasPrefix(trimmed, "git status") && !strings.HasPrefix(trimmed, "git log") {
|
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)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -4,7 +4,6 @@ import (
|
|||||||
"encoding/json"
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
"os"
|
"os"
|
||||||
"os/exec"
|
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
@ -24,7 +23,6 @@ func NewRunner(r *Recipe, client *grok.Client, model string) *Runner {
|
|||||||
func (r *Runner) Run() error {
|
func (r *Runner) Run() error {
|
||||||
fmt.Printf("🍳 Starting recipe: %s v%s\n\n", r.Recipe.Name, r.Recipe.Version)
|
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()
|
workDir := r.resolveWorkDir()
|
||||||
|
|
||||||
var previousResults []string
|
var previousResults []string
|
||||||
@ -50,13 +48,6 @@ func (r *Runner) Run() error {
|
|||||||
r.handleApplyStep(refactorJSONs)
|
r.handleApplyStep(refactorJSONs)
|
||||||
continue
|
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:
|
default:
|
||||||
prompt := fmt.Sprintf(`Recipe Overview:
|
prompt := fmt.Sprintf(`Recipe Overview:
|
||||||
%s
|
%s
|
||||||
@ -92,9 +83,8 @@ Execute this step now. Respond ONLY with the expected output format — no expla
|
|||||||
return nil
|
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 {
|
func (r *Runner) resolveWorkDir() string {
|
||||||
// Start with package_path param or default
|
|
||||||
root := "."
|
root := "."
|
||||||
if v, ok := r.Recipe.ResolvedParams["package_path"]; ok {
|
if v, ok := r.Recipe.ResolvedParams["package_path"]; ok {
|
||||||
if s, ok := v.(string); ok && s != "" {
|
if s, ok := v.(string); ok && s != "" {
|
||||||
@ -102,7 +92,6 @@ func (r *Runner) resolveWorkDir() string {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Expand ~
|
|
||||||
if strings.HasPrefix(root, "~/") {
|
if strings.HasPrefix(root, "~/") {
|
||||||
home, _ := os.UserHomeDir()
|
home, _ := os.UserHomeDir()
|
||||||
root = filepath.Join(home, root[2:])
|
root = filepath.Join(home, root[2:])
|
||||||
@ -110,49 +99,11 @@ func (r *Runner) resolveWorkDir() string {
|
|||||||
root, _ = os.UserHomeDir()
|
root, _ = os.UserHomeDir()
|
||||||
}
|
}
|
||||||
|
|
||||||
// Make absolute
|
abs, _ := filepath.Abs(root)
|
||||||
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
|
|
||||||
}
|
|
||||||
return abs
|
return abs
|
||||||
}
|
}
|
||||||
|
|
||||||
// discoverFiles now takes the resolved absolute path
|
// discoverFiles — uses the resolved workDir (your current signature)
|
||||||
// discoverFiles now takes the resolved workDir
|
|
||||||
func (r *Runner) discoverFiles(workDir string) []string {
|
func (r *Runner) discoverFiles(workDir string) []string {
|
||||||
var files []string
|
var files []string
|
||||||
|
|
||||||
@ -298,100 +249,3 @@ func createUnifiedPatch(changes []FileChange, patchPath string) error {
|
|||||||
}
|
}
|
||||||
return nil
|
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))
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user