From 00508b9636b71df103611f71a10ec7c825acdf85 Mon Sep 17 00:00:00 2001
From: Keith Patrick <keithy@consultant.com>
Date: Mon, 9 Mar 2026 03:32:19 +0000
Subject: [PATCH] Add explicit LC_* vars and PICOCLAW_* prefix blocklist

- Add explicit LC_* vars to default allowlist (locale vars)
- Remove prefix matching for allowlist (use explicit only)
- Add isBlocked() function for blocklist prefix matching
- All PICOCLAW_* vars now blocked from LLM override
- Update config and docs
---
 docs/tools_configuration.md |  9 ++++++---
 pkg/config/config.go        |  2 +-
 pkg/tools/shell/env.go      | 39 +++++++++++++++++++++++++++++++++++--
 3 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/docs/tools_configuration.md b/docs/tools_configuration.md
index f1d11dfb5..8e2e9f752 100644
--- a/docs/tools_configuration.md
+++ b/docs/tools_configuration.md
@@ -58,13 +58,15 @@ The exec tool sanitizes the environment passed to child processes:
 
 1. **Default allowlist** — Only these variables are inherited from the parent process:
    - `PATH`, `HOME`, `USER`, `LANG`, `SHELL`, `TERM`, `PWD`, `OLDPWD`, `HOSTNAME`, `LOGNAME`, `TZ`, `DISPLAY`, `TMPDIR`, `EDITOR`, `PAGER`
-   - Plus: `PICOCLAW_CONFIG`, `PICOCLAW_HOME`, `PICOCLAW_SERVICE_NAME`, `PICOCLAW_EXE`, `PICOCLAW_WORKSPACE`
    - Plus: `HTTP_PROXY`, `HTTPS_PROXY`, `NO_PROXY`
-   - Plus any variable starting with `LC_`
+   - Plus locale: `LC_ALL`, `LC_CTYPE`, `LC_MESSAGES`, `LC_MONETARY`, `LC_NUMERIC`, `LC_TIME`, `LC_PAPER`, `LC_NAME`, `LC_ADDRESS`, `LC_TELEPHONE`, `LC_MEASUREMENT`, `LC_IDENTIFICATION`, `LC_COLLATE`
+   - Plus: `PICOCLAW_HOME`, `PICOCLAW_CONFIG`, `PICOCLAW_AGENT_WORKSPACE`, `PICOCLAW_EXE`, `PICOCLAW_SERVICE_NAME`, `PICOCLAW_EXEC_TIME`, `PICOCLAW_EXEC_TIMEOUT`
 
 2. **Config env_set** — Variables from config are merged (can override inherited values)
 
-3. **LLM env injection** — The LLM can inject additional variables per-call via the `env` parameter:
+3. **Config env_allowlist** — Additional explicit variable names to allow (extends default)
+
+4. **LLM env injection** — The LLM can inject additional variables per-call via the `env` parameter:
 
    ```json
    {
@@ -81,6 +83,7 @@ The exec tool sanitizes the environment passed to child processes:
    **Blocked variables** — The LLM cannot override these sensitive variables:
    - `PATH`, `HOME`, `USER`, `LOGNAME`, `SHELL`
    - `LD_PRELOAD`, `LD_LIBRARY_PATH`, `LD_AUDIT`, `LD_DEBUG`
+   - All `PICOCLAW_*` variables
 
 ### Default Blocked Command Patterns
 
diff --git a/pkg/config/config.go b/pkg/config/config.go
index 296412056..a4e1bf51d 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -646,7 +646,7 @@ type ExecConfig struct {
 	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)
 	EnvSet              map[string]string `                                 json:"env_set"`          // env vars to set for all exec commands
-	EnvAllowlist        []string          `                                 json:"env_allowlist"`   // additional env vars to allow (extends default)
+	EnvAllowlist        []string          `                                 json:"env_allowlist"`   // additional env vars to allow (extends default) - use explicit names, not wildcards
 }
 
 type SkillsToolsConfig struct {
diff --git a/pkg/tools/shell/env.go b/pkg/tools/shell/env.go
index 4fdc8e29d..41c4f4dc3 100644
--- a/pkg/tools/shell/env.go
+++ b/pkg/tools/shell/env.go
@@ -35,11 +35,27 @@ var DefaultEnvAllowlist = map[string]bool{
 	"HTTP_PROXY":   true,
 	"HTTPS_PROXY":  true,
 	"NO_PROXY":     true,
+
+	// Locale
+	"LC_ALL":          true,
+	"LC_CTYPE":        true,
+	"LC_MESSAGES":     true,
+	"LC_MONETARY":     true,
+	"LC_NUMERIC":      true,
+	"LC_TIME":         true,
+	"LC_PAPER":        true,
+	"LC_NAME":         true,
+	"LC_ADDRESS":      true,
+	"LC_TELEPHONE":    true,
+	"LC_MEASUREMENT":  true,
+	"LC_IDENTIFICATION": true,
+	"LC_COLLATE":      true,
 }
 
 // defaultEnvAllowPrefixes are env var prefixes that are always allowed.
+// Currently empty - all allowed vars are explicit in DefaultEnvAllowlist.
 var defaultEnvAllowPrefixes = []string{
-	"LC_",
+	// Currently empty - all allowed vars are explicit
 }
 
 // LLMBlocklist is the set of environment variable names that the LLM
@@ -119,6 +135,25 @@ func WithAllowedEnv(envSet map[string]string, extraAllowlist []string) map[strin
 	return result
 }
 
+// LLMBlocklistPrefixes are env var prefixes that the LLM cannot override.
+var LLMBlocklistPrefixes = []string{
+	"PICOCLAW_",
+}
+
+// isBlocked returns true if the key is in the blocklist or matches a blocked prefix.
+func isBlocked(key string) bool {
+	norm := envKey(key)
+	if LLMBlocklist[norm] {
+		return true
+	}
+	for _, prefix := range LLMBlocklistPrefixes {
+		if strings.HasPrefix(norm, prefix) {
+			return true
+		}
+	}
+	return false
+}
+
 // MergeEnvVars merges multiple env sources into a final []string for exec.Cmd.Env.
 // baseEnv is the cached map from AllowedEnv.
 // envSet provides explicit key=value pairs (config, not filtered).
@@ -141,7 +176,7 @@ func MergeEnvVars(baseEnv map[string]string, envSet, extraEnv map[string]string)
 	// Merge extraEnv (LLM-provided) - filtered by blocklist
 	if extraEnv != nil {
 		for k, v := range extraEnv {
-			if LLMBlocklist[envKey(k)] {
+			if isBlocked(k) {
 				continue // Skip blocked vars
 			}
 			vars[envKey(k)] = v