feat(tools): shell tool security hardening

config/config.example.json

@@ -341,9 +341,12 @@
     },
     "exec": {
       "enabled": true,
-      "enable_deny_patterns": true,
-      "custom_deny_patterns": null,
-      "custom_allow_patterns": null
+      "risk_threshold": "medium",
+      "risk_overrides": {},
+      "arg_profiles": {},
+      "arg_modifiers": {},
+      "env_allowlist": [],
+      "env_set": {}
     },
     "skills": {
       "enabled": true,
@@ -423,4 +426,4 @@
     "host": "127.0.0.1",
     "port": 18790
   }
-}
+}

docs/tools_configuration.md

@@ -45,43 +45,188 @@ Web tools are used for web search and fetching.
 
 ## Exec Tool
 
-The exec tool is used to execute shell commands.
+The exec tool executes shell commands using an in-process POSIX interpreter with
+AST-based risk classification, environment sanitization, and file-access sandboxing.
 
-| Config                 | Type  | Default | Description                                |
-| ---------------------- | ----- | ------- | ------------------------------------------ |
-| `enable_deny_patterns` | bool  | true    | Enable default dangerous command blocking  |
-| `custom_deny_patterns` | array | []      | Custom deny patterns (regular expressions) |
+### Configuration
 
-### Functionality
+| Config           | Type   | Default    | Description                                                                     |
+| ---------------- | ------ | ---------- | ------------------------------------------------------------------------------- |
+| `risk_threshold` | string | `"medium"` | Maximum allowed risk level: `"low"`, `"medium"`, `"high"`, `"critical"`         |
+| `risk_overrides` | object | `{}`       | Per-command base risk level (command name → level); modifiers can still elevate |
+| `arg_profiles`   | object | `{}`       | Per-command argv normalization rules used before argument modifier matching      |
+| `arg_modifiers`  | object | `{}`       | Per-command argument patterns that adjust risk level                            |
+| `env_allowlist`  | array  | `[]`       | Extra environment variables to expose (extends built-in defaults)               |
+| `env_set`        | object | `{}`       | Explicit `VAR=value` pairs injected into every command                          |
 
-- **`enable_deny_patterns`**: Set to `false` to completely disable the default dangerous command blocking patterns
-- **`custom_deny_patterns`**: Add custom deny regex patterns; commands matching these will be blocked
+### Risk Classification
 
-### Default Blocked Command Patterns
+Every command is parsed into an AST before execution. Each resolved binary is
+looked up in a built-in risk table with four levels:
 
-By default, PicoClaw blocks the following dangerous commands:
+| Level      | Meaning                               | Examples                           |
+| ---------- | ------------------------------------- | ---------------------------------- |
+| `low`      | Read-only / informational             | `echo`, `cat`, `ls`, `date`        |
+| `medium`   | Writes files but limited blast radius | `cp`, `mv`, `mkdir`, `tee`         |
+| `high`     | System-wide side effects              | `apt`, `brew`, `docker`, `mount`   |
+| `critical` | Destructive / privilege escalation    | `sudo`, `rm -rf`, `shutdown`, `dd` |
 
-- Delete commands: `rm -rf`, `del /f/q`, `rmdir /s`
-- Disk operations: `format`, `mkfs`, `diskpart`, `dd if=`, writing to `/dev/sd*`
-- System operations: `shutdown`, `reboot`, `poweroff`
-- Command substitution: `$()`, `${}`, backticks
-- Pipe to shell: `| sh`, `| bash`
-- Privilege escalation: `sudo`, `chmod`, `chown`
-- Process control: `pkill`, `killall`, `kill -9`
-- Remote operations: `curl | sh`, `wget | sh`, `ssh`
-- Package management: `apt`, `yum`, `dnf`, `npm install -g`, `pip install --user`
-- Containers: `docker run`, `docker exec`
-- Git: `git push`, `git force`
-- Other: `eval`, `source *.sh`
+Commands with a risk level **above** `risk_threshold` are blocked before execution.
+
+#### Argument modifiers
+
+Some commands change risk depending on their arguments. For example, `rm` is
+classified as `high` by default, and `rm -rf` is treated as `critical`.
+
+You can add custom argument modifiers via config. Each entry lists tokens that
+must all be present (order-independent) and the resulting level:
+
+```json
+{
+  "arg_modifiers": {
+    "curl": [{ "args": ["--upload-file"], "level": "high" }],
+    "git": [{ "args": ["push", "--force"], "level": "critical" }]
+  }
+}
+```
+
+The **highest matching** modifier wins (built-in and custom are merged).
+
+#### Argument profiles
+
+Argument profiles normalize argv before modifier matching. This is useful when a
+tool accepts multiple flag syntaxes for the same semantic operation, such as
+`curl -XPOST`, `curl -X POST`, and `curl --request=POST`.
+
+Each command profile can enable:
+
+- `split_combined_short`: split grouped flags like `-rf` into `-r`, `-f`
+- `split_long_equals`: split `--flag=value` into `--flag`, `value`
+- `short_attached_value_flags`: split attached short-value forms like `-XPOST`
+- `separate_value_flags`: normalize the token after a flag like `-X post`
+
+Supported value transforms are:
+
+- `identity`: keep the value unchanged
+- `upper`: uppercase the value
+- `lower`: lowercase the value
+
+Example:
+
+```json
+{
+  "arg_profiles": {
+    "curl": {
+      "split_combined_short": true,
+      "split_long_equals": true,
+      "short_attached_value_flags": {
+        "-X": "upper",
+        "-d": "identity",
+        "-T": "identity"
+      },
+      "separate_value_flags": {
+        "-X": "upper",
+        "--request": "upper",
+        "-d": "identity",
+        "--data": "identity",
+        "-T": "identity",
+        "--upload-file": "identity"
+      }
+    }
+  }
+}
+```
+
+Built-in profiles are applied first; config profiles extend or override the
+command's flag maps.
+
+#### Precedence
+
+The final risk level is computed as:
+
+1. **Base level**: `risk_overrides` entry if present, else built-in table, else `medium`.
+2. **Modifiers**: All matching argument modifiers (built-in + custom) are scanned.
+   The highest level that exceeds the base is applied. Modifiers can only elevate,
+   never lower.
+
+This means `"risk_overrides": {"rm": "medium"}` allows plain `rm` at the `medium`
+threshold, but `rm -rf` is still elevated to `critical` by the built-in modifier.
+
+#### Shell wrappers
+
+Shell interpreters (`sh`, `bash`, `zsh`, `dash`, `fish`, `ksh`, `csh`, `tcsh`,
+`powershell`, `pwsh`, `cmd`) are classified as `critical` because they can execute
+arbitrary nested commands that bypass the risk classifier (e.g., `sh -c 'rm -rf /'`).
+
+### Environment Sanitization
+
+The shell interpreter runs with a sanitized environment. Only a safe allowlist
+of variables is exposed (e.g., `PATH`, `HOME`, `LANG`, `TERM`).
+
+- **`env_allowlist`**: Extend the defaults with additional variable names.
+- **`env_set`**: Inject fixed `VAR=value` pairs (overrides real env).
+
+### File-Access Sandboxing
+
+When `restrict_to_workspace` is enabled (the default), the interpreter's
+`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
+
+The cron tool creates its own `ExecTool` via `NewExecToolWithConfig` (with `nil`
+bus), so scheduled commands go through the same risk classifier, env
+sanitization, and sandbox as agent-originated commands. Because there is no bus,
+cron-executed commands always run synchronously regardless of the `background`
+parameter.
+
+### Background Execution
+
+When the LLM passes `background: true`, the exec tool launches the command in a
+goroutine and immediately returns a confirmation to the agent. The result is
+delivered asynchronously via the `AsyncCallback` provided by the tool registry.
+
+If no callback is available (e.g. cron-created instances using
+`NewExecToolWithConfig`), `background: true` falls through to synchronous
+execution.
 
 ### Configuration Example
 
 ```json
 {
   "tools": {
     "exec": {
-      "enable_deny_patterns": true,
-      "custom_deny_patterns": ["\\brm\\s+-r\\b", "\\bkillall\\s+python"]
+      "risk_threshold": "medium",
+      "risk_overrides": {
+        "ffmpeg": "low",
+        "terraform": "critical"
+      },
+      "arg_profiles": {
+        "curl": {
+          "split_combined_short": true,
+          "split_long_equals": true,
+          "short_attached_value_flags": {
+            "-X": "upper",
+            "-d": "identity"
+          },
+          "separate_value_flags": {
+            "-X": "upper",
+            "--request": "upper",
+            "-d": "identity",
+            "--data": "identity"
+          }
+        }
+      },
+      "arg_modifiers": {
+        "curl": [{ "args": ["--upload-file"], "level": "high" }]
+      },
+      "env_allowlist": ["GOPATH", "JAVA_HOME"],
+      "env_set": {
+        "NODE_ENV": "production"
+      }
     }
   }
 }
@@ -213,7 +358,7 @@ All configuration options can be overridden via environment variables with the f
 For example:
 
 - `PICOCLAW_TOOLS_WEB_BRAVE_ENABLED=true`
-- `PICOCLAW_TOOLS_EXEC_ENABLE_DENY_PATTERNS=false`
+- `PICOCLAW_TOOLS_EXEC_RISK_THRESHOLD=high`
 - `PICOCLAW_TOOLS_CRON_EXEC_TIMEOUT_MINUTES=10`
 - `PICOCLAW_TOOLS_MCP_ENABLED=true`
 

go.mod

@@ -28,6 +28,7 @@ require (
 	golang.org/x/time v0.14.0
 	google.golang.org/protobuf v1.36.11
 	modernc.org/sqlite v1.46.1
+	mvdan.cc/sh/v3 v3.12.0
 )
 
 require (

go.sum

@@ -40,6 +40,8 @@ github.com/coder/websocket v1.8.14/go.mod h1:NX3SzP+inril6yawo5CQXx8+fk145lPDC6p
 github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc=
 github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
+github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s=
+github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
@@ -56,6 +58,8 @@ github.com/gdamore/tcell/v2 v2.13.8 h1:Mys/Kl5wfC/GcC5Cx4C2BIQH9dbnhnkPgS9/wF3Rl
 github.com/gdamore/tcell/v2 v2.13.8/go.mod h1:+Wfe208WDdB7INEtCsNrAN6O2m+wsTPk1RAovjaILlo=
 github.com/github/copilot-sdk/go v0.1.23 h1:uExtO/inZQndCZMiSAA1hvXINiz9tqo/MZgQzFzurxw=
 github.com/github/copilot-sdk/go v0.1.23/go.mod h1:GdwwBfMbm9AABLEM3x5IZKw4ZfwCYxZ1BgyytmZenQ0=
+github.com/go-quicktest/qt v1.101.0 h1:O1K29Txy5P2OK0dGo59b7b0LR6wKfIhttaAhHUyn7eI=
+github.com/go-quicktest/qt v1.101.0/go.mod h1:14Bz/f7NwaXPtdYEgzsx46kqSxVwTbzVZsDC26tQJow=
 github.com/go-redis/redis/v8 v8.11.4/go.mod h1:2Z2wHZXdQpCDXEGzqMockDpNyYvi2l4Pxt6RJr792+w=
 github.com/go-resty/resty/v2 v2.6.0/go.mod h1:PwvJS6hvaPkjtjNg9ph+VrSD92bi5Zq73w/BIH7cC3Q=
 github.com/go-resty/resty/v2 v2.17.1 h1:x3aMpHK1YM9e4va/TMDRlusDDoZiQ+ViDu/WpA6xTM4=
@@ -113,8 +117,9 @@ github.com/klauspost/cpuid/v2 v2.3.0 h1:S4CRMLnYUhGeDFDqkGriYKdfoFlDnMtqTiI/sFzh
 github.com/klauspost/cpuid/v2 v2.3.0/go.mod h1:hqwkgyIinND0mEev00jJYCxPNVRVXFQeu1XKlok6oO0=
 github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
 github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
-github.com/kr/pretty v0.3.0 h1:WgNl7dwNpEZ6jJ9k1snq4pZsg7DOEN8hP9Xw0Tsjwk0=
 github.com/kr/pretty v0.3.0/go.mod h1:640gp4NfQd8pI5XOwp5fnNeVWj67G7CFk/SaSQn7NBk=
+github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
+github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=
 github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
@@ -165,8 +170,9 @@ github.com/rivo/tview v0.42.0/go.mod h1:cSfIYfhpSGCjp3r/ECJb+GKS7cGJnqV8vfjQPwoX
 github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ=
 github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88=
 github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc=
-github.com/rogpeppe/go-internal v1.9.0 h1:73kH8U+JUqXU8lRuOHeVHaa/SZPifC7BkcraZVejAe8=
 github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs=
+github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ=
+github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc=
 github.com/rs/xid v1.6.0/go.mod h1:7XoLgs4eV+QndskICGsho+ADou8ySMSjJKDIan90Nz0=
 github.com/rs/zerolog v1.34.0 h1:k43nTLIwcTVQAncfCw4KZ2VY6ukYoZaBPNOE8txlOeY=
 github.com/rs/zerolog v1.34.0/go.mod h1:bJsvje4Z08ROH4Nhs5iH600c3IkWhwp44iRc54W6wYQ=
@@ -383,5 +389,7 @@ modernc.org/strutil v1.2.1 h1:UneZBkQA+DX2Rp35KcM69cSsNES9ly8mQWD71HKlOA0=
 modernc.org/strutil v1.2.1/go.mod h1:EHkiggD70koQxjVdSBM3JKM7k6L0FbGE5eymy9i3B9A=
 modernc.org/token v1.1.0 h1:Xl7Ap9dKaEs5kLoOQeQmPWevfnk/DM5qcLcYlA8ys6Y=
 modernc.org/token v1.1.0/go.mod h1:UGzOrNV1mAFSEB63lOFHIpNRUVMvYTc6yu1SMY/XTDM=
+mvdan.cc/sh/v3 v3.12.0 h1:ejKUR7ONP5bb+UGHGEG/k9V5+pRVIyD+LsZz7o8KHrI=
+mvdan.cc/sh/v3 v3.12.0/go.mod h1:Se6Cj17eYSn+sNooLZiEUnNNmNxg0imoYlTu4CyaGyg=
 rsc.io/qr v0.2.0 h1:6vBLea5/NRMVTz8V66gipeLycZMl/+UlFmk8DvqQ6WY=
 rsc.io/qr v0.2.0/go.mod h1:IF+uZjkb9fqyeF/4tlBoynqmQxUoPfWEKh921coOuXs=

pkg/agent/instance.go

@@ -78,13 +78,6 @@ func NewAgentInstance(
 	if cfg.Tools.IsToolEnabled("list_dir") {
 		toolsRegistry.Register(tools.NewListDirTool(workspace, readRestrict, allowReadPaths))
 	}
-	if cfg.Tools.IsToolEnabled("exec") {
-		execTool, err := tools.NewExecToolWithConfig(workspace, restrict, cfg)
-		if err != nil {
-			log.Fatalf("Critical error: unable to initialize exec tool: %v", err)
-		}
-		toolsRegistry.Register(execTool)
-	}
 
 	if cfg.Tools.IsToolEnabled("edit_file") {
 		toolsRegistry.Register(tools.NewEditFileTool(workspace, restrict, allowWritePaths))

pkg/agent/loop.go

@@ -168,6 +168,18 @@ func registerSharedTools(
 			agent.Tools.Register(tools.NewSPITool())
 		}
 
+		// Exec tool — created here (not in NewAgentInstance) for
+		// consistent tool registration alongside other shared tools.
+		if cfg.Tools.IsToolEnabled("exec") {
+			restrict := cfg.Agents.Defaults.RestrictToWorkspace
+			execTool, err := tools.NewExecToolWithConfig(agent.Workspace, restrict, cfg)
+			if err != nil {
+				logger.ErrorCF("agent", "Failed to create exec tool", map[string]any{"error": err.Error()})
+			} else {
+				agent.Tools.Register(execTool)
+			}
+		}
+
 		// Message tool
 		if cfg.Tools.IsToolEnabled("message") {
 			messageTool := tools.NewMessageTool()

pkg/config/config.go

@@ -602,12 +602,36 @@ type CronToolsConfig struct {
 	ExecTimeoutMinutes int `                                 env:"PICOCLAW_TOOLS_CRON_EXEC_TIMEOUT_MINUTES" json:"exec_timeout_minutes"` // 0 means no timeout
 }
 
+// ArgModifierConfig describes an argument pattern that elevates a command's risk.
+type ArgModifierConfig struct {
+	Args  []string `json:"args"`  // tokens that must all be present (order-independent)
+	Level string   `json:"level"` // target risk level: "low"|"medium"|"high"|"critical"
+}
+
+// ArgProfileConfig describes how a command's argv should be normalized before
+// argument-aware risk modifiers are matched.
+type ArgProfileConfig struct {
+	SplitCombinedShort bool              `json:"split_combined_short"`
+	SplitLongEquals    bool              `json:"split_long_equals"`
+	ShortAttachedValue map[string]string `json:"short_attached_value_flags"`
+	SeparateValueFlags map[string]string `json:"separate_value_flags"`
+}
+
 type ExecConfig struct {
-	ToolConfig          `         envPrefix:"PICOCLAW_TOOLS_EXEC_"`
-	EnableDenyPatterns  bool     `                                 env:"PICOCLAW_TOOLS_EXEC_ENABLE_DENY_PATTERNS"  json:"enable_deny_patterns"`
-	CustomDenyPatterns  []string `                                 env:"PICOCLAW_TOOLS_EXEC_CUSTOM_DENY_PATTERNS"  json:"custom_deny_patterns"`
-	CustomAllowPatterns []string `                                 env:"PICOCLAW_TOOLS_EXEC_CUSTOM_ALLOW_PATTERNS" json:"custom_allow_patterns"`
-	TimeoutSeconds      int      `                                 env:"PICOCLAW_TOOLS_EXEC_TIMEOUT_SECONDS"       json:"timeout_seconds"` // 0 means use default (60s)
+	ToolConfig    `                               envPrefix:"PICOCLAW_TOOLS_EXEC_"`
+	RiskThreshold string                         `                                 json:"risk_threshold" env:"PICOCLAW_TOOLS_EXEC_RISK_THRESHOLD"` // "low"|"medium"|"high"|"critical"; default "medium"
+	RiskOverrides map[string]string              `                                 json:"risk_overrides" env:"PICOCLAW_TOOLS_EXEC_RISK_OVERRIDES"` // command → level override
+	ArgProfiles   map[string]ArgProfileConfig    `                                 json:"arg_profiles"`                                            // command → argv normalization profile (extends built-ins)
+	ArgModifiers  map[string][]ArgModifierConfig `                                 json:"arg_modifiers"  env:"PICOCLAW_TOOLS_EXEC_ARG_MODIFIERS"`  // command → argument-aware risk adjustments (extends built-ins)
+	EnvAllowlist  []string                       `                                 json:"env_allowlist"  env:"PICOCLAW_TOOLS_EXEC_ENV_ALLOWLIST"`  // extra env vars to pass (extends defaults)
+	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"`
+	// 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"`
 }
 
 type SkillsToolsConfig struct {

pkg/config/defaults.go

@@ -385,8 +385,7 @@ func DefaultConfig() *Config {
 				ToolConfig: ToolConfig{
 					Enabled: true,
 				},
-				EnableDenyPatterns: true,
-				TimeoutSeconds:     60,
+				RiskThreshold: "medium",
 			},
 			Skills: SkillsToolsConfig{
 				ToolConfig: ToolConfig{