feat: block exec commands from writing to protected dirs via denyWritePaths

pkg/agent/instance.go

@@ -96,11 +96,13 @@ func NewAgentInstance(
 		toolsRegistry.Register(tools.NewListDirTool(workspace, readRestrict, allowReadPaths, denyReadPaths))
 	}
 	if cfg.Tools.IsToolEnabled("exec") {
-		execTool, err := tools.NewExecToolWithConfig(workspace, restrict, cfg, allowReadPaths)
+		execTool, err := tools.NewExecToolWithDenyPaths(workspace, restrict, [][]*regexp.Regexp{allowReadPaths}, denyWritePaths, cfg)
 		if err != nil {
 			logger.ErrorCF("agent", "Failed to initialize exec tool; continuing without exec",
 				map[string]any{"error": err.Error()})
-		} else {
+			execTool = nil
+		}
+		if execTool != nil {
 			toolsRegistry.Register(execTool)
 		}
 	}

pkg/tools/shell.go

@@ -40,6 +40,7 @@ type ExecTool struct {
 	allowPatterns       []*regexp.Regexp
 	customAllowPatterns []*regexp.Regexp
 	allowedPathPatterns []*regexp.Regexp
+	denyWritePaths    []*regexp.Regexp
 	restrictToWorkspace bool
 	allowRemote         bool
 	sessionManager      *SessionManager
@@ -114,14 +115,24 @@ var (
 )
 
 func NewExecTool(workingDir string, restrict bool, allowPaths ...[]*regexp.Regexp) (*ExecTool, error) {
-	return NewExecToolWithConfig(workingDir, restrict, nil, allowPaths...)
+	return NewExecToolWithDenyPaths(workingDir, restrict, allowPaths, nil, nil)
 }
 
 func NewExecToolWithConfig(
 	workingDir string,
 	restrict bool,
-	config *config.Config,
+	cfg *config.Config,
 	allowPaths ...[]*regexp.Regexp,
+) (*ExecTool, error) {
+	return NewExecToolWithDenyPaths(workingDir, restrict, allowPaths, nil, cfg)
+}
+
+func NewExecToolWithDenyPaths(
+	workingDir string,
+	restrict bool,
+	allowPaths [][]*regexp.Regexp,
+	denyWritePaths []*regexp.Regexp,
+	cfg *config.Config,
 ) (*ExecTool, error) {
 	denyPatterns := make([]*regexp.Regexp, 0)
 	customAllowPatterns := make([]*regexp.Regexp, 0)
@@ -131,8 +142,8 @@ func NewExecToolWithConfig(
 		allowedPathPatterns = allowPaths[0]
 	}
 
-	if config != nil {
-		execConfig := config.Tools.Exec
+	if cfg != nil {
+		execConfig := cfg.Tools.Exec
 		enableDenyPatterns := execConfig.EnableDenyPatterns
 		allowRemote = execConfig.AllowRemote
 		if enableDenyPatterns {
@@ -148,7 +159,6 @@ func NewExecToolWithConfig(
 				}
 			}
 		} else {
-			// If deny patterns are disabled, we won't add any patterns, allowing all commands.
 			fmt.Println("Warning: deny patterns are disabled. All commands will be allowed.")
 		}
 		for _, pattern := range execConfig.CustomAllowPatterns {
@@ -163,8 +173,8 @@ func NewExecToolWithConfig(
 	}
 
 	var timeout time.Duration
-	if config != nil && config.Tools.Exec.TimeoutSeconds > 0 {
-		timeout = time.Duration(config.Tools.Exec.TimeoutSeconds) * time.Second
+	if cfg != nil && cfg.Tools.Exec.TimeoutSeconds > 0 {
+		timeout = time.Duration(cfg.Tools.Exec.TimeoutSeconds) * time.Second
 	}
 
 	return &ExecTool{
@@ -174,6 +184,7 @@ func NewExecToolWithConfig(
 		allowPatterns:       nil,
 		customAllowPatterns: customAllowPatterns,
 		allowedPathPatterns: allowedPathPatterns,
+		denyWritePaths:      denyWritePaths,
 		restrictToWorkspace: restrict,
 		allowRemote:         allowRemote,
 		sessionManager:      getSessionManager(),
@@ -1033,6 +1044,28 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
 				return "Command blocked by safety guard (dangerous pattern detected)"
 			}
 		}
+
+		// Check deny write paths - block commands that write to protected directories
+		if len(t.denyWritePaths) > 0 {
+			words := strings.Fields(cmd)
+			for i, word := range words {
+				for _, pattern := range t.denyWritePaths {
+					if pattern.MatchString(word) {
+						return fmt.Sprintf("Command blocked: cannot write to %s (access denied)", word)
+					}
+					// Also check path components like "skills" in "mkdir -p skills/my_skill"
+					if i >= 0 && (word == "-p" || word == "-rf" || word == "-r") {
+						continue
+					}
+					pathParts := strings.Split(word, "/")
+					for _, part := range pathParts {
+						if pattern.MatchString(part) {
+							return fmt.Sprintf("Command blocked: cannot write to %s (access denied)", part)
+						}
+					}
+				}
+			}
+		}
 	}
 
 	if len(t.allowPatterns) > 0 {

pkg/tools/shell_test.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"os"
 	"path/filepath"
+	"regexp"
 	"runtime"
 	"strings"
 	"testing"
@@ -1613,3 +1614,54 @@ func TestEncodeKeyTokenWithPtyKeyMode(t *testing.T) {
 		})
 	}
 }
+
+func TestShellTool_DenyWritePaths(t *testing.T) {
+	tests := []struct {
+		name        string
+		command     string
+		denyPaths   []*regexp.Regexp
+		expectBlock bool
+	}{
+		{
+			name:        "mkdir blocked",
+			command:     "mkdir -p skills",
+			denyPaths:   []*regexp.Regexp{regexp.MustCompile(`^skills(/.*)?$`)},
+			expectBlock: true,
+		},
+		{
+			name:        "mkdir -p blocked",
+			command:     "mkdir -p skills/my_skill",
+			denyPaths:   []*regexp.Regexp{regexp.MustCompile(`^skills(/.*)?$`)},
+			expectBlock: true,
+		},
+		{
+			name:        "mkdir allowed",
+			command:     "mkdir -p workspace/data",
+			denyPaths:   []*regexp.Regexp{regexp.MustCompile(`^skills(/.*)?$`)},
+			expectBlock: false,
+		},
+		{
+			name:        "touch skills file blocked",
+			command:     "touch skills/test.txt",
+			denyPaths:   []*regexp.Regexp{regexp.MustCompile(`^skills(/.*)?$`)},
+			expectBlock: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tool, err := NewExecToolWithDenyPaths("", false, nil, tt.denyPaths, nil)
+			require.NoError(t, err)
+			result := tool.Execute(context.Background(), map[string]any{
+				"action":  "run",
+				"command": tt.command,
+			})
+			if tt.expectBlock {
+				require.True(t, result.IsError, "expected block for command: %s", tt.command)
+				require.Contains(t, result.ForLLM, "access denied")
+			} else {
+				require.False(t, result.IsError, "expected allow for command: %s, got: %s", tt.command, result.ForLLM)
+			}
+		})
+	}
+}