harden root-path deletion guard for rm variants

pkg/tools/risk.go

@@ -28,11 +28,11 @@ var destructivePatterns = []*regexp.Regexp{
 	regexp.MustCompile(`\bchown\b.+\s+/`),
 	regexp.MustCompile(`\bclawgo\s+uninstall\b`),
 	regexp.MustCompile(`\bdbt\s+drop\b`),
+	regexp.MustCompile(`\bgit\s+clean\b`),
 }
 
 var moderatePatterns = []*regexp.Regexp{
 	regexp.MustCompile(`\bgit\s+reset\s+--hard\b`),
-	regexp.MustCompile(`\bgit\s+clean\b`),
 	regexp.MustCompile(`\bdocker\s+system\s+prune\b`),
 	regexp.MustCompile(`\bapt(-get)?\s+install\b`),
 	regexp.MustCompile(`\byum\s+install\b`),

pkg/tools/shell.go

@@ -15,7 +15,7 @@ import (
 	"clawgo/pkg/logger"
 )
 
-var blockedRootWipePattern = regexp.MustCompile(`(?i)(^|[;&|\n])\s*rm\s+-rf\s+/\s*($|[;&|\n])`)
+var blockedRootWipePattern = regexp.MustCompile(`(?i)(^|[;&|\n])\s*rm\b[^\n;&|]*\s(?:'/'|"/"|/)(?:\s|$)`)
 
 type ExecTool struct {
 	workingDir          string
@@ -29,15 +29,21 @@ type ExecTool struct {
 }
 
 func NewExecTool(cfg config.ShellConfig, workspace string) *ExecTool {
-	denyPatterns := make([]*regexp.Regexp, 0)
+	denyPatterns := make([]*regexp.Regexp, 0, len(cfg.DeniedCmds))
 	for _, p := range cfg.DeniedCmds {
-		denyPatterns = append(denyPatterns, regexp.MustCompile(`\b`+regexp.QuoteMeta(p)+`\b`))
+		denyPatterns = append(denyPatterns, regexp.MustCompile(`(?i)\b`+regexp.QuoteMeta(p)+`\b`))
+	}
+
+	allowPatterns := make([]*regexp.Regexp, 0, len(cfg.AllowedCmds))
+	for _, p := range cfg.AllowedCmds {
+		allowPatterns = append(allowPatterns, regexp.MustCompile(`(?i)\b`+regexp.QuoteMeta(p)+`\b`))
 	}
 
 	return &ExecTool{
 		workingDir:          workspace,
 		timeout:             cfg.Timeout,
 		denyPatterns:        denyPatterns,
+		allowPatterns:       allowPatterns,
 		restrictToWorkspace: cfg.RestrictPath,
 		sandboxEnabled:      cfg.Sandbox.Enabled,
 		sandboxImage:        cfg.Sandbox.Image,
@@ -149,11 +155,11 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
 	lower := strings.ToLower(cmd)
 
 	if blockedRootWipePattern.MatchString(lower) {
-		return "Command blocked by safety guard (rm -rf / is forbidden)"
+		return "Command blocked by safety guard (removing root path / is forbidden)"
 	}
 
 	for _, pattern := range t.denyPatterns {
-		if pattern.MatchString(lower) {
+		if pattern.MatchString(cmd) {
 			return "Command blocked by safety guard (dangerous pattern detected)"
 		}
 	}
@@ -161,7 +167,7 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
 	if len(t.allowPatterns) > 0 {
 		allowed := false
 		for _, pattern := range t.allowPatterns {
-			if pattern.MatchString(lower) {
+			if pattern.MatchString(cmd) {
 				allowed = true
 				break
 			}
@@ -215,7 +221,7 @@ func (t *ExecTool) SetRestrictToWorkspace(restrict bool) {
 func (t *ExecTool) SetAllowPatterns(patterns []string) error {
 	t.allowPatterns = make([]*regexp.Regexp, 0, len(patterns))
 	for _, p := range patterns {
-		re, err := regexp.Compile(p)
+		re, err := regexp.Compile("(?i)" + p)
 		if err != nil {
 			return fmt.Errorf("invalid allow pattern %q: %w", p, err)
 		}
@@ -254,10 +260,11 @@ func (t *ExecTool) applyRiskGate(command string, force bool) (string, string) {
 		return "Error: destructive command is disabled by policy (tools.shell.risk.allow_destructive=false).", ""
 	}
 
-	if t.riskCfg.RequireDryRun {
+	if t.riskCfg.RequireDryRun && !force {
 		if dryRunCmd, ok := buildDryRunCommand(command); ok {
 			return "Risk gate: dry-run required first. Review output, then execute intentionally with force=true.", dryRunCmd
 		}
+		return "Error: destructive command requires explicit force=true because no dry-run strategy is available.", ""
 	}
 	return "", ""
 }

pkg/tools/shell_test.go

@@ -0,0 +1,119 @@
+package tools
+
+import (
+	"testing"
+
+	"clawgo/pkg/config"
+)
+
+func TestApplyRiskGate_DryRunCanBeBypassedWithForce(t *testing.T) {
+	tool := &ExecTool{riskCfg: config.RiskConfig{
+		Enabled:          true,
+		AllowDestructive: true,
+		RequireDryRun:    true,
+		RequireForceFlag: false,
+	}}
+
+	msg, dryRun := tool.applyRiskGate("git clean -fd", true)
+	if msg != "" || dryRun != "" {
+		t.Fatalf("expected force=true to allow execution after dry-run step, got msg=%q dryRun=%q", msg, dryRun)
+	}
+}
+
+func TestApplyRiskGate_RequiresDryRunWithoutForce(t *testing.T) {
+	tool := &ExecTool{riskCfg: config.RiskConfig{
+		Enabled:          true,
+		AllowDestructive: true,
+		RequireDryRun:    true,
+		RequireForceFlag: false,
+	}}
+
+	msg, dryRun := tool.applyRiskGate("git clean -fd", false)
+	if msg == "" {
+		t.Fatal("expected dry-run block message")
+	}
+	if dryRun == "" {
+		t.Fatal("expected dry-run command")
+	}
+}
+
+func TestAssessCommandRisk_GitCleanIsDestructive(t *testing.T) {
+	assessment := assessCommandRisk("git clean -fd")
+	if assessment.Level != RiskDestructive {
+		t.Fatalf("expected git clean to be destructive, got %s", assessment.Level)
+	}
+}
+
+func TestNewExecTool_LoadsAllowedCmdsIntoAllowPatterns(t *testing.T) {
+	tool := NewExecTool(config.ShellConfig{AllowedCmds: []string{"echo"}}, ".")
+	if len(tool.allowPatterns) != 1 {
+		t.Fatalf("expected one allow pattern, got %d", len(tool.allowPatterns))
+	}
+}
+
+func TestGuardCommand_BlocksCommandNotInAllowlist(t *testing.T) {
+	tool := NewExecTool(config.ShellConfig{AllowedCmds: []string{"echo"}}, ".")
+	if msg := tool.guardCommand("ls -la", "."); msg == "" {
+		t.Fatal("expected allowlist to block command not in allowed_cmds")
+	}
+
+	if msg := tool.guardCommand("echo hi", "."); msg != "" {
+		t.Fatalf("expected allowed command to pass guard, got %q", msg)
+	}
+}
+
+func TestGuardCommand_AllowlistIsCaseInsensitive(t *testing.T) {
+	tool := NewExecTool(config.ShellConfig{AllowedCmds: []string{"ECHO"}}, ".")
+	if msg := tool.guardCommand("echo hi", "."); msg != "" {
+		t.Fatalf("expected case-insensitive allowlist match, got %q", msg)
+	}
+}
+
+func TestGuardCommand_DenylistIsCaseInsensitive(t *testing.T) {
+	tool := NewExecTool(config.ShellConfig{DeniedCmds: []string{"RM"}}, ".")
+	if msg := tool.guardCommand("rm -f tmp.txt", "."); msg == "" {
+		t.Fatal("expected case-insensitive denylist match to block command")
+	}
+}
+
+func TestApplyRiskGate_RequireDryRunWithoutStrategyStillBlocks(t *testing.T) {
+	tool := &ExecTool{riskCfg: config.RiskConfig{
+		Enabled:          true,
+		AllowDestructive: true,
+		RequireDryRun:    true,
+		RequireForceFlag: false,
+	}}
+
+	msg, dryRun := tool.applyRiskGate("rm -rf tmp", false)
+	if msg == "" {
+		t.Fatal("expected destructive command without dry-run strategy to be blocked")
+	}
+	if dryRun != "" {
+		t.Fatalf("expected no dry-run command for rm -rf, got %q", dryRun)
+	}
+}
+
+func TestSetAllowPatterns_IsCaseInsensitive(t *testing.T) {
+	tool := &ExecTool{}
+	if err := tool.SetAllowPatterns([]string{`^ECHO\b`}); err != nil {
+		t.Fatalf("SetAllowPatterns returned error: %v", err)
+	}
+
+	if msg := tool.guardCommand("echo hi", "."); msg != "" {
+		t.Fatalf("expected case-insensitive allow pattern to match, got %q", msg)
+	}
+}
+
+func TestGuardCommand_BlocksRootWipeVariants(t *testing.T) {
+	tool := &ExecTool{}
+	cases := []string{
+		"rm -rf /",
+		"rm -fr /",
+		"rm --no-preserve-root -rf /",
+	}
+	for _, c := range cases {
+		if msg := tool.guardCommand(c, "."); msg == "" {
+			t.Fatalf("expected root wipe variant to be blocked: %s", c)
+		}
+	}
+}