feat(security): enhance sandboxing and risk classification for shell commands; add Windows-specific tests and deprecate old config fields

Signed-off-by: Boris Bliznioukov <blib@mail.com>

docs/tools_configuration.md

@@ -120,7 +120,11 @@ of variables is exposed (e.g., `PATH`, `HOME`, `LANG`, `TERM`).
 ### File-Access Sandboxing
 
 When `restrict_to_workspace` is enabled (the default), the interpreter's
-`OpenHandler` blocks reads and writes outside the configured workspace directory.
+`OpenHandler` blocks reads and writes outside the configured workspace directory for shell-managed redirections (`>`, `<`, `>>`).
+
+> NOTE: This is not a general filesystem sandbox. External programs invoked by the
+> shell can still perform arbitrary file I/O via their own syscalls; only
+> shell-level redirections are constrained by `OpenHandler`.
 
 ### Cron Integration
 

pkg/config/config.go

@@ -616,8 +616,10 @@ type ExecConfig struct {
 	EnvSet        map[string]string              `json:"env_set"        env:"PICOCLAW_TOOLS_EXEC_ENV_SET"`        // explicit var=value pairs
 
 	// Deprecated: these fields are ignored. See risk_threshold and risk_overrides.
-	EnableDenyPatterns  bool     `json:"enable_deny_patterns,omitempty"  env:"PICOCLAW_TOOLS_EXEC_ENABLE_DENY_PATTERNS"`
+	EnableDenyPatterns *bool `json:"enable_deny_patterns,omitempty" env:"PICOCLAW_TOOLS_EXEC_ENABLE_DENY_PATTERNS"`
-	CustomDenyPatterns  []string `json:"custom_deny_patterns,omitempty"  env:"PICOCLAW_TOOLS_EXEC_CUSTOM_DENY_PATTERNS"`
+	// Deprecated: these fields are ignored. See risk_threshold and risk_overrides.
+	CustomDenyPatterns []string `json:"custom_deny_patterns,omitempty" env:"PICOCLAW_TOOLS_EXEC_CUSTOM_DENY_PATTERNS"`
+	// Deprecated: these fields are ignored. See risk_threshold and risk_overrides.
 	CustomAllowPatterns []string `json:"custom_allow_patterns,omitempty" env:"PICOCLAW_TOOLS_EXEC_CUSTOM_ALLOW_PATTERNS"`
 }
 

pkg/migrate/sources/openclaw/openclaw_config.go

@@ -546,7 +546,6 @@ func (c *OpenClawConfig) ConvertToPicoClaw(sourceHome string) (*PicoClawConfig,
 			"Auth profiles (API keys, OAuth tokens) not migrated for security - set env vars manually",
 		)
 	}
-
 	return cfg, warnings, nil
 }
 
@@ -1066,9 +1065,5 @@ func (c ToolsConfig) ToStandardTools() config.ToolsConfig {
 		Cron: config.CronToolsConfig{
 			ExecTimeoutMinutes: c.Cron.ExecTimeoutMinutes,
 		},
-		Exec: config.ExecConfig{
-			EnableDenyPatterns: c.Exec.EnableDenyPatterns,
-			CustomDenyPatterns: c.Exec.CustomDenyPatterns,
-		},
 	}
 }

pkg/tools/shell/env.go

@@ -56,15 +56,15 @@ var windowsEnvAllowlist = map[string]bool{
 func BuildSanitizedEnv(extraAllowlist []string, envSet map[string]string) expand.Environ {
 	allowed := make(map[string]bool, len(DefaultEnvAllowlist)+len(extraAllowlist)+len(windowsEnvAllowlist))
 	for k := range DefaultEnvAllowlist {
-		allowed[k] = true
+		allowed[envKey(k)] = true
 	}
 	if runtime.GOOS == "windows" {
 		for k := range windowsEnvAllowlist {
-			allowed[k] = true
+			allowed[envKey(k)] = true
 		}
 	}
 	for _, k := range extraAllowlist {
-		allowed[k] = true
+		allowed[envKey(k)] = true
 	}
 
 	vars := make(map[string]string, len(allowed)+len(envSet))
@@ -74,18 +74,29 @@ func BuildSanitizedEnv(extraAllowlist []string, envSet map[string]string) expand
 		if !ok {
 			continue
 		}
-		if allowed[k] || isAllowedPrefix(k) {
+		norm := envKey(k)
-			vars[k] = v
+		if allowed[norm] || isAllowedPrefix(norm) {
+			vars[norm] = v
 		}
 	}
 
 	for k, v := range envSet {
-		vars[k] = v
+		vars[envKey(k)] = v
 	}
 
 	return &sanitizedEnv{vars: vars}
 }
 
+// envKey normalizes an environment variable name. On Windows, where env
+// vars are case-insensitive, it uppercases the key so that "Path" and
+// "PATH" map to the same entry. On other platforms it's a no-op.
+func envKey(k string) string {
+	if runtime.GOOS == "windows" {
+		return strings.ToUpper(k)
+	}
+	return k
+}
+
 func isAllowedPrefix(name string) bool {
 	for _, prefix := range defaultEnvAllowPrefixes {
 		if strings.HasPrefix(name, prefix) {
@@ -101,7 +112,7 @@ type sanitizedEnv struct {
 }
 
 func (e *sanitizedEnv) Get(name string) expand.Variable {
-	val, ok := e.vars[name]
+	val, ok := e.vars[envKey(name)]
 	if !ok {
 		return expand.Variable{}
 	}

pkg/tools/shell/env_windows_test.go

@@ -0,0 +1,52 @@
+package shell
+
+import (
+	"testing"
+)
+
+func TestBuildSanitizedEnv_WindowsCaseInsensitive(t *testing.T) {
+	// On Windows the OS typically stores "Path" not "PATH".
+	// Verify that mixed-case inherited vars still pass the allowlist.
+	t.Setenv("Path", `C:\Windows\system32`)
+
+	env := BuildSanitizedEnv(nil, nil)
+
+	v := env.Get("PATH")
+	if !v.IsSet() {
+		t.Fatal("expected PATH to be present when OS provides 'Path'")
+	}
+	if v.Str != `C:\Windows\system32` {
+		t.Errorf("PATH = %q, want %q", v.Str, `C:\Windows\system32`)
+	}
+
+	// Also verify lookup with original casing works.
+	v2 := env.Get("Path")
+	if !v2.IsSet() {
+		t.Fatal("expected Get('Path') to resolve via case-insensitive lookup")
+	}
+}
+
+func TestBuildSanitizedEnv_WindowsEnvSetCaseInsensitive(t *testing.T) {
+	env := BuildSanitizedEnv(nil, map[string]string{
+		"path": `C:\custom\bin`,
+	})
+
+	v := env.Get("PATH")
+	if !v.IsSet() {
+		t.Fatal("expected PATH to be set via lowercase 'path' envSet key")
+	}
+	if v.Str != `C:\custom\bin` {
+		t.Errorf("PATH = %q, want %q", v.Str, `C:\custom\bin`)
+	}
+}
+
+func TestBuildSanitizedEnv_WindowsExtraAllowlistCaseInsensitive(t *testing.T) {
+	t.Setenv("my_custom_var", "hello")
+
+	env := BuildSanitizedEnv([]string{"MY_CUSTOM_VAR"}, nil)
+
+	v := env.Get("my_custom_var")
+	if !v.IsSet() {
+		t.Fatal("expected my_custom_var to be found via case-insensitive allowlist")
+	}
+}

pkg/tools/shell/risk.go

@@ -446,31 +446,62 @@ func IsAllowed(level, threshold RiskLevel) bool {
 	return level <= threshold
 }
 
-// BlockedCommandError formats a structured error message for the LLM.
+// BlockedError is returned when a command is blocked by the risk classifier.
-func BlockedCommandError(args []string, level, threshold RiskLevel, reason string) string {
+type BlockedError struct {
-	cmd := ""
+	Command   string
-	if len(args) > 0 {
+	Level     RiskLevel
-		cmd = args[0]
+	Threshold RiskLevel
-		if len(args) > 1 {
+	Reason    string
-			end := len(args)
+}
-			if end > 5 {
-				end = 5
-			}
-			for _, a := range args[1:end] {
-				cmd += " " + a
-			}
-			if len(args) > 5 {
-				cmd += " ..."
-			}
-		}
-	}
 
+func (e *BlockedError) Error() string {
 	return fmt.Sprintf(
 		"Command blocked by risk classifier: command=%q risk_level=%s threshold=%s reason=%s",
-		cmd, level, threshold, reason,
+		e.Command, e.Level, e.Threshold, e.Reason,
 	)
 }
 
+// BlockedCommandError formats a structured error message for the LLM.
+// Deprecated: use BlockedError directly.
+func BlockedCommandError(args []string, level, threshold RiskLevel, reason string) string {
+	return (&BlockedError{
+		Command:   formatCommand(args),
+		Level:     level,
+		Threshold: threshold,
+		Reason:    reason,
+	}).Error()
+}
+
+// NewBlockedError constructs a BlockedError from a command's args.
+func NewBlockedError(args []string, level, threshold RiskLevel, reason string) *BlockedError {
+	return &BlockedError{
+		Command:   formatCommand(args),
+		Level:     level,
+		Threshold: threshold,
+		Reason:    reason,
+	}
+}
+
+func formatCommand(args []string) string {
+	if len(args) == 0 {
+		return ""
+	}
+	cmd := args[0]
+	if len(args) > 1 {
+		end := len(args)
+		if end > 5 {
+			end = 5
+		}
+		for _, a := range args[1:end] {
+			cmd += " " + a
+		}
+		if len(args) > 5 {
+			cmd += " ..."
+		}
+	}
+	return cmd
+}
+
 // baseCommand extracts the basename from a command path.
 // On Windows, it additionally lowercases the name and strips known
 // executable extensions (.exe, .cmd, .bat, .com) so that

pkg/tools/shell/runner.go

@@ -3,6 +3,7 @@ package shell
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -97,14 +98,17 @@ func Run(ctx context.Context, cfg RunConfig) RunResult {
 	}
 
 	if err != nil {
-		if runCtx.Err() == context.DeadlineExceeded {
+		if runCtx.Err() != nil {
-			msg := fmt.Sprintf("Command timed out after %v", cfg.Timeout)
+			if runCtx.Err() == context.DeadlineExceeded {
-			return RunResult{Output: msg, IsError: true}
+				msg := fmt.Sprintf("Command timed out after %v", cfg.Timeout)
+				return RunResult{Output: msg, IsError: true}
+			}
+			return RunResult{Output: "Command canceled", IsError: true}
 		}
 
-		errStr := err.Error()
+		var blocked *BlockedError
-		if strings.Contains(errStr, "Command blocked by risk classifier") {
+		if errors.As(err, &blocked) {
-			return RunResult{Output: errStr, IsError: true}
+			return RunResult{Output: blocked.Error(), IsError: true}
 		}
 
 		output += fmt.Sprintf("\nExit code: %v", err)
@@ -140,8 +144,7 @@ func riskExecHandler(
 
 			level := ClassifyCommand(args, overrides, extraMods)
 			if !IsAllowed(level, threshold) {
-				reason := "command risk exceeds configured threshold"
+				return NewBlockedError(args, level, threshold, "command risk exceeds configured threshold")
-				return fmt.Errorf("%s", BlockedCommandError(args, level, threshold, reason))
 			}
 
 			return next(ctx, args)

pkg/tools/shell/runner_test.go

@@ -2,10 +2,12 @@ package shell
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"path/filepath"
 	"runtime"
 	"strings"
+	"syscall"
 	"testing"
 	"time"
 
@@ -91,6 +93,10 @@ func TestRun_BlocksCommandSubstitution(t *testing.T) {
 }
 
 func TestRun_Timeout(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("requires Unix sleep command")
+	}
+
 	result := Run(context.Background(), RunConfig{
 		Command:       "sleep 60",
 		Dir:           t.TempDir(),
@@ -106,7 +112,78 @@ func TestRun_Timeout(t *testing.T) {
 	}
 }
 
+func TestRun_TimeoutKillsBackgroundChild(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("Unix-only: relies on kill -0 for process liveness check")
+	}
+
+	tmpDir := t.TempDir()
+	pidFile := filepath.Join(tmpDir, "child.pid")
+
+	// Spawn a background sh that writes its real OS PID, then execs sleep.
+	// The foreground also blocks on sleep so the interpreter stays alive
+	// until the timeout fires. sh is normally risk=critical so we
+	// downgrade it via an override for this test.
+	cmd := fmt.Sprintf(
+		`/bin/sh -c 'echo $$ > %s; exec sleep 300' & sleep 300`,
+		pidFile,
+	)
+
+	result := Run(context.Background(), RunConfig{
+		Command:       cmd,
+		Dir:           tmpDir,
+		Timeout:       2 * time.Second,
+		RiskThreshold: RiskMedium,
+		RiskOverrides: map[string]string{"sh": "low", "/bin/sh": "low"},
+	})
+
+	if !result.IsError {
+		t.Fatal("expected timeout error")
+	}
+	if !strings.Contains(result.Output, "timed out") {
+		t.Errorf("expected 'timed out' in output: %s", result.Output)
+	}
+
+	// Read the PID that was written by the background sh process.
+	raw, err := os.ReadFile(pidFile)
+	if err != nil {
+		// PID file might not have been flushed before timeout — that's fine,
+		// it just means the child never started and there's nothing to check.
+		t.Skipf("PID file not written (child may not have started): %v", err)
+	}
+	pidStr := strings.TrimSpace(string(raw))
+	if pidStr == "" {
+		t.Skip("PID file empty — child may not have started")
+	}
+
+	var pid int
+	if _, err := fmt.Sscanf(pidStr, "%d", &pid); err != nil {
+		t.Fatalf("failed to parse PID %q: %v", pidStr, err)
+	}
+
+	// Give the OS a moment to reap the child.
+	time.Sleep(200 * time.Millisecond)
+
+	proc, err := os.FindProcess(pid)
+	if err != nil {
+		// Process already gone — exactly what we want.
+		return
+	}
+
+	// Signal 0 checks liveness without actually killing.
+	if err := proc.Signal(syscall.Signal(0)); err == nil {
+		t.Errorf("background child (PID %d) still alive after timeout — process leak", pid)
+		// Best-effort cleanup so we don't leave a zombie.
+		_ = proc.Kill()
+	}
+	// err != nil means the process is gone — success.
+}
+
 func TestRun_WorkingDir(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("requires Unix cat command")
+	}
+
 	tmpDir := t.TempDir()
 	testFile := filepath.Join(tmpDir, "test.txt")
 	os.WriteFile(testFile, []byte("test content"), 0o644)
@@ -143,6 +220,10 @@ func TestRun_ParseError(t *testing.T) {
 }
 
 func TestRun_StderrCapture(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("requires Unix shell redirections")
+	}
+
 	result := Run(context.Background(), RunConfig{
 		Command:       "echo stdout_msg; echo stderr_msg >&2",
 		Dir:           t.TempDir(),
@@ -159,6 +240,10 @@ func TestRun_StderrCapture(t *testing.T) {
 }
 
 func TestRun_HighThresholdAllowsRm(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("requires Unix rm command")
+	}
+
 	tmpDir := t.TempDir()
 	testFile := filepath.Join(tmpDir, "delete_me.txt")
 	os.WriteFile(testFile, []byte("bye"), 0o644)
@@ -179,6 +264,10 @@ func TestRun_HighThresholdAllowsRm(t *testing.T) {
 }
 
 func TestRun_EnvSanitization(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("requires Unix env command")
+	}
+
 	t.Setenv("OPENAI_API_KEY", "sk-secret-test")
 	t.Setenv("PATH", os.Getenv("PATH"))
 
@@ -201,6 +290,10 @@ func TestRun_EnvSanitization(t *testing.T) {
 }
 
 func TestRun_PipelineCommand(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("requires Unix wc command")
+	}
+
 	result := Run(context.Background(), RunConfig{
 		Command:       "echo 'line1\nline2\nline3' | wc -l",
 		Dir:           t.TempDir(),
@@ -214,6 +307,10 @@ func TestRun_PipelineCommand(t *testing.T) {
 }
 
 func TestRun_DevNullRedirection(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("requires /dev/null")
+	}
+
 	result := Run(context.Background(), RunConfig{
 		Command:       "echo hello 2>/dev/null",
 		Dir:           t.TempDir(),
@@ -229,6 +326,10 @@ func TestRun_DevNullRedirection(t *testing.T) {
 }
 
 func TestRun_RiskOverrides(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("requires Unix rm command and /dev/null")
+	}
+
 	// Override rm to low so it passes with threshold=medium.
 	result := Run(context.Background(), RunConfig{
 		Command:       "rm nonexistent_file_xyz 2>/dev/null; echo done",

pkg/tools/shell/runner_windows_test.go

@@ -0,0 +1,122 @@
+package shell
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+)
+
+func TestRun_Timeout_Windows(t *testing.T) {
+	// ping with a high count effectively blocks like sleep on Unix.
+	result := Run(context.Background(), RunConfig{
+		Command:       "ping -n 60 127.0.0.1",
+		Dir:           t.TempDir(),
+		Timeout:       500 * time.Millisecond,
+		RiskThreshold: RiskMedium,
+		RiskOverrides: map[string]string{"ping": "low"},
+	})
+
+	if !result.IsError {
+		t.Fatal("expected timeout error")
+	}
+	if !strings.Contains(result.Output, "timed out") {
+		t.Errorf("expected 'timed out' in output: %s", result.Output)
+	}
+}
+
+func TestRun_WorkingDir_Windows(t *testing.T) {
+	tmpDir := t.TempDir()
+	testFile := filepath.Join(tmpDir, "test.txt")
+	os.WriteFile(testFile, []byte("test content"), 0o644)
+
+	result := Run(context.Background(), RunConfig{
+		Command:       "cmd.exe /c type test.txt",
+		Dir:           tmpDir,
+		Timeout:       5 * time.Second,
+		RiskThreshold: RiskHigh, // cmd.exe is risk=critical
+		RiskOverrides: map[string]string{"cmd.exe": "low"},
+	})
+
+	if result.IsError {
+		t.Fatalf("expected success: %s", result.Output)
+	}
+	if !strings.Contains(result.Output, "test content") {
+		t.Errorf("expected 'test content' in output: %s", result.Output)
+	}
+}
+
+func TestRun_HighThresholdAllowsDel_Windows(t *testing.T) {
+	tmpDir := t.TempDir()
+	testFile := filepath.Join(tmpDir, "delete_me.txt")
+	os.WriteFile(testFile, []byte("bye"), 0o644)
+
+	result := Run(context.Background(), RunConfig{
+		Command:       "cmd.exe /c del delete_me.txt",
+		Dir:           tmpDir,
+		Timeout:       5 * time.Second,
+		RiskThreshold: RiskHigh,
+		RiskOverrides: map[string]string{"cmd.exe": "low"},
+	})
+
+	if result.IsError {
+		t.Fatalf("with threshold=high, del should be allowed: %s", result.Output)
+	}
+	if _, err := os.Stat(testFile); err == nil {
+		t.Error("file should have been deleted")
+	}
+}
+
+func TestRun_EnvSanitization_Windows(t *testing.T) {
+	t.Setenv("OPENAI_API_KEY", "sk-secret-test")
+	t.Setenv("PATH", os.Getenv("PATH"))
+
+	result := Run(context.Background(), RunConfig{
+		Command:       "cmd.exe /c set",
+		Dir:           t.TempDir(),
+		Timeout:       5 * time.Second,
+		RiskThreshold: RiskHigh,
+		RiskOverrides: map[string]string{"cmd.exe": "low"},
+	})
+
+	if result.IsError {
+		t.Fatalf("expected set command to succeed: %s", result.Output)
+	}
+	if strings.Contains(result.Output, "OPENAI_API_KEY") {
+		t.Error("OPENAI_API_KEY should not be in child environment")
+	}
+	if !strings.Contains(result.Output, "PATH=") {
+		t.Error("PATH should be in child environment")
+	}
+}
+
+func TestRun_NulRedirection_Windows(t *testing.T) {
+	result := Run(context.Background(), RunConfig{
+		Command:       "echo hello 2>NUL",
+		Dir:           t.TempDir(),
+		Timeout:       5 * time.Second,
+		Restrict:      true,
+		WorkspaceDir:  t.TempDir(),
+		RiskThreshold: RiskMedium,
+	})
+
+	if result.IsError && strings.Contains(result.Output, "sandbox") {
+		t.Errorf("NUL should not be blocked: %s", result.Output)
+	}
+}
+
+func TestRun_RiskOverrides_Windows(t *testing.T) {
+	result := Run(context.Background(), RunConfig{
+		Command:       "cmd.exe /c del nonexistent_file_xyz 2>NUL & echo done",
+		Dir:           t.TempDir(),
+		Timeout:       5 * time.Second,
+		RiskThreshold: RiskMedium,
+		RiskOverrides: map[string]string{"cmd.exe": "low"},
+	})
+
+	if result.IsError && strings.Contains(result.Output, "blocked") {
+		t.Errorf("cmd.exe should be allowed with override: %s", result.Output)
+	}
+}

pkg/tools/shell/sandbox.go

@@ -6,13 +6,15 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"runtime"
+	"strings"
 
 	"mvdan.cc/sh/v3/interp"
 )
 
-// SafePaths are kernel pseudo-devices that are always safe to open,
+// safePaths are kernel pseudo-devices that are always safe to open,
 // regardless of workspace restriction.
-var SafePaths = map[string]bool{
+var safePaths = map[string]bool{
 	"/dev/null":    true,
 	"/dev/zero":    true,
 	"/dev/random":  true,
@@ -22,6 +24,19 @@ var SafePaths = map[string]bool{
 	"/dev/stderr":  true,
 }
 
+// isSafePath reports whether path is a platform-appropriate pseudo-device
+// that should always be accessible regardless of sandbox restrictions.
+func isSafePath(path string) bool {
+	if safePaths[path] {
+		return true
+	}
+	// On Windows, NUL (case-insensitive) is the equivalent of /dev/null.
+	if runtime.GOOS == "windows" && strings.EqualFold(path, "NUL") {
+		return true
+	}
+	return false
+}
+
 // SandboxedOpenHandler returns an interp.OpenHandlerFunc that restricts
 // shell redirections (>, <, >>) to files within the workspace directory.
 //
@@ -41,7 +56,7 @@ func SandboxedOpenHandler(workspaceDir string) interp.OpenHandlerFunc {
 	}
 
 	return func(ctx context.Context, path string, flag int, perm os.FileMode) (io.ReadWriteCloser, error) {
-		if SafePaths[path] {
+		if isSafePath(path) {
 			return os.OpenFile(path, flag, perm)
 		}
 

pkg/tools/shell/sandbox_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"testing"
 	"time"
@@ -40,9 +41,16 @@ func TestSandboxedOpenHandler_AllowsSafePaths(t *testing.T) {
 	workspace := t.TempDir()
 	handler := SandboxedOpenHandler(workspace)
 
-	f, err := handler(context.Background(), "/dev/null", os.O_WRONLY, 0)
+	// Pick a platform-appropriate safe path that exists in the
+	// sandbox's safe-path list and is openable on the current OS.
+	safePath := "/dev/null"
+	if runtime.GOOS == "windows" {
+		safePath = "NUL"
+	}
+
+	f, err := handler(context.Background(), safePath, os.O_WRONLY, 0)
 	if err != nil {
-		t.Fatalf("expected /dev/null to be allowed: %v", err)
+		t.Fatalf("expected %s to be allowed: %v", safePath, err)
 	}
 	f.Close()
 }

pkg/tools/shell_tool.go

@@ -71,6 +71,18 @@ func NewExecToolWithConfig(workingDir string, restrict bool, cfg *config.Config)
 }
 
 func warnDeprecatedExecConfig(cfg config.ExecConfig) {
+	if cfg.EnableDenyPatterns != nil {
+		if !*cfg.EnableDenyPatterns {
+			fmt.Println("Warning: 'enable_deny_patterns: false' is deprecated and ignored. " +
+				"Previously this disabled all command filtering. The new risk-based system " +
+				"is now always active (default threshold=medium). " +
+				"To allow all commands, set 'risk_threshold: critical'.")
+		} else {
+			fmt.Println("Warning: 'enable_deny_patterns' is deprecated and ignored. " +
+				"Command filtering is now always active via the risk-based classifier. " +
+				"Remove this field from your config.")
+		}
+	}
 	if len(cfg.CustomDenyPatterns) > 0 {
 		fmt.Println("Warning: 'custom_deny_patterns' is deprecated and ignored. " +
 			"Use 'risk_overrides' to adjust per-command risk levels.")

pkg/tools/shell_tool_test.go

@@ -1,10 +1,16 @@
 package tools
 
 import (
+	"bytes"
 	"context"
+	"fmt"
+	"os"
+	"strings"
 	"sync"
 	"testing"
 	"time"
+
+	"github.com/sipeed/picoclaw/pkg/config"
 )
 
 func TestExecTool_SyncExecution(t *testing.T) {
@@ -143,3 +149,122 @@ func TestExecTool_ImplementsAsyncTool(t *testing.T) {
 
 	var _ AsyncTool = tool // compile-time check
 }
+
+// captureStdout runs fn and returns whatever it wrote to os.Stdout.
+func captureStdout(t *testing.T, fn func()) string {
+	t.Helper()
+
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	old := os.Stdout
+	os.Stdout = w
+
+	fn()
+
+	w.Close()
+	os.Stdout = old
+
+	var buf bytes.Buffer
+	buf.ReadFrom(r)
+	return buf.String()
+}
+
+func boolPtr(b bool) *bool { return &b }
+
+func TestWarnDeprecatedExecConfig_EnableDenyPatternsFalse(t *testing.T) {
+	out := captureStdout(t, func() {
+		warnDeprecatedExecConfig(config.ExecConfig{
+			EnableDenyPatterns: boolPtr(false),
+		})
+	})
+
+	if !strings.Contains(out, "enable_deny_patterns: false") {
+		t.Errorf("expected warning about 'enable_deny_patterns: false', got: %s", out)
+	}
+	if !strings.Contains(out, "risk_threshold: critical") {
+		t.Errorf("expected migration hint to 'risk_threshold: critical', got: %s", out)
+	}
+}
+
+func TestWarnDeprecatedExecConfig_EnableDenyPatternsTrue(t *testing.T) {
+	out := captureStdout(t, func() {
+		warnDeprecatedExecConfig(config.ExecConfig{
+			EnableDenyPatterns: boolPtr(true),
+		})
+	})
+
+	if !strings.Contains(out, "enable_deny_patterns") {
+		t.Errorf("expected deprecation warning, got: %s", out)
+	}
+	if !strings.Contains(out, "Remove this field") {
+		t.Errorf("expected removal hint, got: %s", out)
+	}
+}
+
+func TestWarnDeprecatedExecConfig_NilNoWarning(t *testing.T) {
+	out := captureStdout(t, func() {
+		warnDeprecatedExecConfig(config.ExecConfig{})
+	})
+
+	if strings.Contains(out, "enable_deny_patterns") {
+		t.Errorf("expected no warning when field is absent, got: %s", out)
+	}
+}
+
+func TestWarnDeprecatedExecConfig_CustomPatterns(t *testing.T) {
+	out := captureStdout(t, func() {
+		warnDeprecatedExecConfig(config.ExecConfig{
+			CustomDenyPatterns:  []string{"rm"},
+			CustomAllowPatterns: []string{"ls"},
+		})
+	})
+
+	if !strings.Contains(out, "custom_deny_patterns") {
+		t.Errorf("expected custom_deny_patterns warning, got: %s", out)
+	}
+	if !strings.Contains(out, "custom_allow_patterns") {
+		t.Errorf("expected custom_allow_patterns warning, got: %s", out)
+	}
+}
+
+func TestWarnDeprecatedExecConfig_AllDeprecatedFields(t *testing.T) {
+	out := captureStdout(t, func() {
+		warnDeprecatedExecConfig(config.ExecConfig{
+			EnableDenyPatterns:  boolPtr(false),
+			CustomDenyPatterns:  []string{"rm"},
+			CustomAllowPatterns: []string{"ls"},
+		})
+	})
+
+	// All three warnings should fire.
+	for _, want := range []string{
+		"enable_deny_patterns: false",
+		"custom_deny_patterns",
+		"custom_allow_patterns",
+	} {
+		if !strings.Contains(out, want) {
+			t.Errorf("expected warning containing %q, got: %s", want, out)
+		}
+	}
+}
+
+func TestNewExecToolWithConfig_EnableDenyPatternsFalseWarning(t *testing.T) {
+	out := captureStdout(t, func() {
+		cfg := &config.Config{}
+		cfg.Tools.Exec.EnableDenyPatterns = boolPtr(false)
+		_, err := NewExecToolWithConfig(t.TempDir(), false, cfg)
+		if err != nil {
+			t.Fatal(err)
+		}
+	})
+
+	if !strings.Contains(out, "enable_deny_patterns: false") {
+		t.Errorf("expected warning in NewExecToolWithConfig output: %s", out)
+	}
+}
+
+// Suppress unused import lint for fmt (used by captureStdout indirectly).
+var _ = fmt.Sprintf