diff --git a/claudetool/bash.go b/claudetool/bash.go index 78d3653ddf0c8a0c86cd61e8a88de1c62f059fd0..9f998ea58a7888a351613be04f25da7e48883009 100644 --- a/claudetool/bash.go +++ b/claudetool/bash.go @@ -107,6 +107,10 @@ Use background for servers/demos that need to stay running. MUST set slow_ok=true for potentially slow commands: builds, downloads, installs, tests, or any other substantive operation. +Avoid overly destructive cleanup commands. Commands that could delete .git +directories, home directories, or use broad wildcards require explicit paths. +Confirm with the user before running destructive operations. + To change the working directory persistently, use the change_dir tool. IMPORTANT: Keep commands concise. The command input must be less than 60k tokens. diff --git a/claudetool/bashkit/bashkit.go b/claudetool/bashkit/bashkit.go index 676cc0b875474f3d69cfd2f31f7fc8a0eaa60436..4abf724a4df0615f9fd5af3bf25773c34bb71e25 100644 --- a/claudetool/bashkit/bashkit.go +++ b/claudetool/bashkit/bashkit.go @@ -11,6 +11,7 @@ import ( var checks = []func(*syntax.CallExpr) error{ noBlindGitAdd, + noDangerousRmRf, } // Process-level checks that track state across calls @@ -103,6 +104,122 @@ func WillRunGitCommit(bashScript string) (bool, error) { return willCommit, nil } +// noDangerousRmRf checks for rm -rf commands that could delete critical directories. +// It rejects patterns that could delete .git directories, home directories (~, $HOME), +// or root directories. +func noDangerousRmRf(cmd *syntax.CallExpr) error { + if hasDangerousRmRf(cmd) { + return fmt.Errorf("permission denied: this rm command could delete critical data (.git, home directory, or root). If you really need to run this command, spell out the full path explicitly (no wildcards, ~, or $HOME). Consider confirming with the user before running destructive cleanup commands") + } + return nil +} + +// hasDangerousRmRf checks if an rm command could delete critical directories. +func hasDangerousRmRf(cmd *syntax.CallExpr) bool { + if len(cmd.Args) < 1 { + return false + } + + // Check if the command is rm + firstArg := cmd.Args[0].Lit() + if firstArg != "rm" { + return false + } + + // Check if -r or -R is present (recursive) + hasRecursive := false + hasForce := false + for _, arg := range cmd.Args[1:] { + lit := arg.Lit() + // Handle combined flags like -rf, -fr, -Rf, etc. + if strings.HasPrefix(lit, "-") && !strings.HasPrefix(lit, "--") { + if strings.ContainsAny(lit, "rR") { + hasRecursive = true + } + if strings.Contains(lit, "f") { + hasForce = true + } + } + if lit == "--recursive" { + hasRecursive = true + } + if lit == "--force" { + hasForce = true + } + } + + // Only check for dangerous paths if it's a recursive and forced rm + if !hasRecursive || !hasForce { + return false + } + + // Check arguments for dangerous patterns + for _, arg := range cmd.Args[1:] { + lit := arg.Lit() + // Skip flags + if strings.HasPrefix(lit, "-") { + continue + } + + // Check for .git directory patterns + if lit == ".git" || strings.HasSuffix(lit, "/.git") || + strings.Contains(lit, ".git/") || strings.Contains(lit, ".git ") { + return true + } + + // Check for home directory patterns + if lit == "~" || lit == "~/" || strings.HasPrefix(lit, "~/") { + return true + } + + // Check for root directory + if lit == "/" { + return true + } + + // Check for wildcards that could match .git + if lit == ".*" || strings.HasSuffix(lit, "/.*") { + return true + } + + // Check for broad wildcards at dangerous locations + if lit == "*" || lit == "/*" { + return true + } + } + + // Also check if the argument uses variable expansion (like $HOME) + // We need to walk the AST more carefully for this + for _, arg := range cmd.Args[1:] { + if containsHomeVariable(arg) { + return true + } + } + + return false +} + +// containsHomeVariable checks if a word contains $HOME or ${HOME} expansion +func containsHomeVariable(word *syntax.Word) bool { + for _, part := range word.Parts { + switch p := part.(type) { + case *syntax.ParamExp: + if p.Param != nil && p.Param.Value == "HOME" { + return true + } + case *syntax.DblQuoted: + for _, inner := range p.Parts { + if pe, ok := inner.(*syntax.ParamExp); ok { + if pe.Param != nil && pe.Param.Value == "HOME" { + return true + } + } + } + } + } + return false +} + // noBlindGitAdd checks for git add commands that blindly add all files. // It rejects patterns like 'git add -A', 'git add .', 'git add --all', 'git add *'. func noBlindGitAdd(cmd *syntax.CallExpr) error { diff --git a/claudetool/bashkit/bashkit_test.go b/claudetool/bashkit/bashkit_test.go index 0a894fa605070f89ac73ba4816a2b7c73f70a158..9592d111bf01784feb55a54693c46e6b579ad313 100644 --- a/claudetool/bashkit/bashkit_test.go +++ b/claudetool/bashkit/bashkit_test.go @@ -419,6 +419,168 @@ func TestHasSketchWipBranchChanges(t *testing.T) { } } +func TestDangerousRmRf(t *testing.T) { + tests := []struct { + name string + script string + wantErr bool + errMatch string + }{ + // Dangerous rm -rf commands that should be blocked + { + name: "rm -rf .git", + script: "rm -rf .git", + wantErr: true, + errMatch: "could delete critical data", + }, + { + name: "rm -rf with path ending in .git", + script: "rm -rf /path/to/.git", + wantErr: true, + errMatch: "could delete critical data", + }, + { + name: "rm -rf ~ (home directory)", + script: "rm -rf ~", + wantErr: true, + errMatch: "could delete critical data", + }, + { + name: "rm -rf ~/ (home directory with slash)", + script: "rm -rf ~/", + wantErr: true, + errMatch: "could delete critical data", + }, + { + name: "rm -rf ~/path", + script: "rm -rf ~/Documents", + wantErr: true, + errMatch: "could delete critical data", + }, + { + name: "rm -rf $HOME", + script: "rm -rf $HOME", + wantErr: true, + errMatch: "could delete critical data", + }, + { + name: "rm -rf ${HOME}", + script: "rm -rf ${HOME}", + wantErr: true, + errMatch: "could delete critical data", + }, + { + name: "rm -rf / (root)", + script: "rm -rf /", + wantErr: true, + errMatch: "could delete critical data", + }, + { + name: "rm -rf .* (hidden files wildcard)", + script: "rm -rf .*", + wantErr: true, + errMatch: "could delete critical data", + }, + { + name: "rm -rf * (all files wildcard)", + script: "rm -rf *", + wantErr: true, + errMatch: "could delete critical data", + }, + { + name: "rm -rf /* (root wildcard)", + script: "rm -rf /*", + wantErr: true, + errMatch: "could delete critical data", + }, + { + name: "rm -rf with separate flags", + script: "rm -r -f .git", + wantErr: true, + errMatch: "could delete critical data", + }, + { + name: "rm -Rf .git (capital R)", + script: "rm -Rf .git", + wantErr: true, + errMatch: "could delete critical data", + }, + { + name: "rm --recursive --force .git", + script: "rm --recursive --force .git", + wantErr: true, + errMatch: "could delete critical data", + }, + { + name: "rm -rf path/.*/", + script: "rm -rf path/.*", + wantErr: true, + errMatch: "could delete critical data", + }, + // Safe rm commands that should be allowed + { + name: "rm -rf specific directory", + script: "rm -rf /tmp/build", + wantErr: false, + }, + { + name: "rm -rf node_modules", + script: "rm -rf node_modules", + wantErr: false, + }, + { + name: "rm -rf specific file", + script: "rm -rf /tmp/file.txt", + wantErr: false, + }, + { + name: "rm without recursive (safe)", + script: "rm -f .git", + wantErr: false, + }, + { + name: "rm without force (safe)", + script: "rm -r .git", + wantErr: false, + }, + { + name: "rm single file", + script: "rm file.txt", + wantErr: false, + }, + { + name: "rm -rf with quoted $HOME (literal string)", + script: "rm -rf '$HOME'", + wantErr: false, // single quotes make it literal + }, + // Complex commands + { + name: "multiline with dangerous rm", + script: "echo cleaning && rm -rf .git && echo done", + wantErr: true, + errMatch: "could delete critical data", + }, + { + name: "multiline with safe rm", + script: "echo cleaning && rm -rf /tmp/build && echo done", + wantErr: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := Check(tc.script) + if (err != nil) != tc.wantErr { + t.Errorf("Check() error = %v, wantErr %v", err, tc.wantErr) + return + } + if tc.wantErr && err != nil && !strings.Contains(err.Error(), tc.errMatch) { + t.Errorf("Check() error message = %v, want containing %v", err, tc.errMatch) + } + }) + } +} + func TestEdgeCases(t *testing.T) { tests := []struct { name string