feat(security): add environment sanitization for exec tool

[keithy]

📝 Description

This is an urgent requirement as the entire environment secrets and all is being passed through to LLM and the exec tool, secrets and all.

This PR aims to start looking at hardening the exec tool. I have diced up #1097 to pull out and refine the Environmment sanitisation feature.

🗣️ Type of Change

[X] 🐞 Bug fix
[ ] ✨ New feature
[x] 📖 Documentation update
[ ] ⚡ Code refactoring

🤖 AI Code Generation

[x] 👨‍💻 50/50 Human/AI written

🔗 Related Issues

• feat(tools): shell tool security hardening #1097

🧪 Test Environment

Hardware: Rock64 (ARM64)
OS: Debian/Ubuntu (Linux) armbian, mise
Channels: CLI / Local Daemon

☑️ Checklist

[x] My code/docs follow the style of this project
[x] I have performed a self-review of my own changes
[x] I have updated the documentation

Summary

• Add configurable env allowlist
• Add PICOCLAW_* env vars (HOME, CONFIG, AGENT_WORKSPACE, EXE, SERVICE_NAME, EXEC_TIME, EXEC_TIMEOUT) passed to child processes
• Block LLM from overriding sensitive vars (PATH, HOME, LD_PRELOAD, PICOCLAW_*, etc.)
• Efficient env handling with os.Getenv() lookups
• Explicit LC_* locale vars in allowlist
• The LLM is told that it can set the env in its tool call.

Next Steps

Round 2: Make the actual tool itself easier to swap/plugin different implementations, then we can try out different options.

Round 3: Implement a secure shell execution tool, resitant to injections etc.
My plan will be to banish so insecure and heavy weight bash, and ask the LLM to use the lean and mean execline instead.

[CLAassistant]

All committers have signed the CLA.

[keithy]

Standard:

HOME=/home/infra
LANG=en_US.UTF-8
LC_ALL=en_US.UTF-8
LC_MESSAGES=en_US.UTF-8
LOGNAME=infra
OLDPWD=/infra/code/picoclaw
PATH=...
PWD=/infra
SHELL=/bin/bash
TERM=xterm-256color
USER=infra

PICOCLAW:

PICOCLAW_AGENT_WORKSPACE=/infra
PICOCLAW_CONFIG=/home/infra/.picoclaw/config.json
PICOCLAW_EXEC_TIME=2026-03-09T04:54:12Z
PICOCLAW_EXEC_TIMEOUT=1m0s
PICOCLAW_EXE=/infra/code/picoclaw/build/picoclaw-linux-arm64
PICOCLAW_HOME=/home/infra/.picoclaw
PICOCLAW_SERVICE_NAME=picoclaw

[nikolasdehor]

This is an important security improvement. Sanitizing the environment passed to child processes prevents accidental credential leakage (API keys, tokens, etc.) to LLM-invoked commands. The approach is well-structured.

Detailed feedback:

Architecture (positive):

• Separating env logic into pkg/tools/shell/env.go is clean
• The three-layer merge (cached base -> config env_set -> LLM extra) with different trust levels is the right design
• LLM blocklist preventing override of PATH/HOME/LD_PRELOAD is essential
• Windows case-insensitive handling via envKey is a good detail

Concerns:

1. envSet map mutation -- WithAllowedEnv modifies the envSet map in-place (result = envSet, then writes to result). If the caller retains a reference to envSet, they will see unexpected mutations. Either document this or copy the map first:

result := make(map[string]string, len(envSet))
for k, v := range envSet { result[k] = v }

2. Same issue in WithPicoclawEnvVars -- it also mutates the input envSet map.

3. Missing tests -- for a security-critical feature, unit tests are essential:

   • Verify that API keys/tokens from parent env are NOT passed to children
   • Verify that LLM cannot override PATH, LD_PRELOAD
   • Verify that config env_set overrides inherited env
   • Verify that env_allowlist correctly extends the default list
   • Verify Windows case-insensitive behavior

4. http_proxy/https_proxy lowercase variants -- the allowlist has HTTP_PROXY and HTTPS_PROXY, but many tools also use lowercase http_proxy/https_proxy. On Linux (case-sensitive), these would be stripped. Consider adding the lowercase variants.

5. GOPATH, GOROOT, NODE_PATH -- tools invoked by the exec command may need these. Users can add them via env_allowlist, but it might be worth mentioning in the docs.

6. Breaking change potential -- existing setups that rely on env vars being passed through to exec commands (e.g., OPENAI_API_KEY used by Python scripts called from exec) will silently break after this change. Consider:

   • A migration note in the PR description
   • A config option to opt out of sanitization (env_sanitize: false)
   • At minimum, a log warning when env vars are stripped

Overall this is the right direction for security hardening. Please add tests and address the map mutation issue.

[keithy]

Thanks @nikolasdehor for the excellent feedback

1. Fixed map mutation in WithAllowedEnv and WithPicoclawEnvVars - using maps.Clone
2. Added lowercase http_proxy/https_proxy/no_proxy to allowlist
3. Added debug logging of final list of env keys passed at exec time
4. Added unit tests

Held off on the following

• env_sanitize: false option: Just curious as to the use case
• GOPATH/GOROOT/NODE_PATH docs: Not used in codebase - would like to understand the uses.

Skipped

• Windows case-insensitive test: (mock runtime.GOOS?)

Tests Added

• TestWithAllowedEnv_DefaultAllowlist
• TestWithAllowedEnv_ExtraAllowlist
• TestWithAllowedEnv_EnvSetOverride
• TestMergeEnvVars_LLMBlocked
• TestMergeEnvVars_ConfigEnvNotFiltered
• TestMergeEnvVars_PICOCLAWNotOverridable

[sipeed-bot]

@keithy Hi! This PR has been inactive for a while now. If there's no update in the next 7 days, it will be closed automatically. If the environment sanitization feature is still being worked on, just drop a comment!