Split env handling: FilterByAllowlist at init, MergeEnvVars at exec

- Add FilterByAllowlist: filters os.Environ() by allowlist (run once at init)
- Add MergeEnvVars: merges cached env with envSet and extraEnv (no re-filtering)
- cachedEnv now stores map[string]string, pre-merged with PICOCLAW_*
- PICOCLAW_* vars now survive to child processes
This commit is contained in:
Keith Patrick 2026-03-09 02:25:25 +00:00
parent e0ffee8339
commit 7b82cac4df
2 changed files with 85 additions and 5 deletions

View file

@ -24,7 +24,7 @@ type ExecTool struct {
allowPatterns []*regexp.Regexp
customAllowPatterns []*regexp.Regexp
restrictToWorkspace bool
cachedEnv []string // cached sanitized env from os.Environ() at init
cachedEnv map[string]string // cached sanitized env map from os.Environ() at init
}
var (
@ -151,6 +151,15 @@ func NewExecToolWithConfig(workingDir string, restrict bool, config *config.Conf
// Ensure PICOCLAW_* vars are set for child processes
envSet = shell.WithPicoclawEnvVars(envSet, workingDir)
// Build cached env: filter inherited env by allowlist, then merge with config envSet
filteredBase := shell.FilterByAllowlist(os.Environ(), envAllowlist)
// Pre-merge envSet into cachedEnv so PICOCLAW_* vars are preserved
// (MergeEnvVars returns []string, so we merge maps manually)
cachedEnv := filteredBase
for k, v := range envSet {
cachedEnv[k] = v
}
return &ExecTool{
workingDir: workingDir,
timeout: timeout,
@ -158,7 +167,7 @@ func NewExecToolWithConfig(workingDir string, restrict bool, config *config.Conf
allowPatterns: nil,
customAllowPatterns: customAllowPatterns,
restrictToWorkspace: restrict,
cachedEnv: shell.BuildSanitizedEnv(os.Environ(), envAllowlist, envSet, nil),
cachedEnv: cachedEnv,
}, nil
}
@ -258,9 +267,9 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult
"PICOCLAW_EXEC_TIMEOUT": t.timeout.String(),
}
// Use sanitized environment - strips secrets, prevents env-based attacks
// Pass extraEnv from LLM to apply blocklist filtering
cmd.Env = shell.BuildSanitizedEnv(t.cachedEnv, nil, execTimeEnv, extraEnv)
// Use sanitized environment - merge cached env with exec time vars and LLM extra env
// Note: cachedEnv is NOT re-filtered - PICOCLAW_* vars are preserved
cmd.Env = shell.MergeEnvVars(t.cachedEnv, execTimeEnv, extraEnv)
if cwd != "" {
cmd.Dir = cwd

View file

@ -78,6 +78,77 @@ var windowsEnvAllowlist = map[string]bool{
"HOMEPATH": true,
}
// FilterByAllowlist filters the inherited environment to only allowlisted variables.
// This should only be used at init time to create the cached environment.
func FilterByAllowlist(baseEnv []string, extraAllowlist []string) map[string]string {
if baseEnv == nil {
baseEnv = os.Environ()
}
allowed := make(map[string]bool, len(DefaultEnvAllowlist)+len(extraAllowlist)+len(windowsEnvAllowlist))
for k := range DefaultEnvAllowlist {
allowed[envKey(k)] = true
}
if runtime.GOOS == "windows" {
for k := range windowsEnvAllowlist {
allowed[envKey(k)] = true
}
}
for _, k := range extraAllowlist {
allowed[envKey(k)] = true
}
vars := make(map[string]string)
for _, entry := range baseEnv {
k, v, ok := strings.Cut(entry, "=")
if !ok {
continue
}
norm := envKey(k)
if allowed[norm] || isAllowedPrefix(norm) {
vars[norm] = v
}
}
return vars
}
// MergeEnvVars merges multiple env sources into a final []string for exec.Cmd.Env.
// baseEnv is NOT filtered - it's assumed to already be sanitized (e.g., cachedEnv).
// envSet provides explicit key=value pairs (config, not filtered).
// extraEnv provides additional key=value pairs from LLM (filtered by blocklist).
func MergeEnvVars(baseEnv map[string]string, envSet, extraEnv map[string]string) []string {
vars := make(map[string]string, len(baseEnv)+len(envSet)+len(extraEnv))
// Start with base env (already sanitized)
for k, v := range baseEnv {
vars[envKey(k)] = v
}
// Add envSet (config-provided, not filtered)
if envSet != nil {
for k, v := range envSet {
vars[envKey(k)] = v
}
}
// Merge extraEnv (LLM-provided) - filtered by blocklist
if extraEnv != nil {
for k, v := range extraEnv {
if LLMBlocklist[envKey(k)] {
continue // Skip blocked vars
}
vars[envKey(k)] = v
}
}
// Convert to []string for exec.Cmd.Env
result := make([]string, 0, len(vars))
for k, v := range vars {
result = append(result, k+"="+v)
}
return result
}
// BuildSanitizedEnv constructs a sanitized environment []string suitable for
// exec.Cmd.Env. It filters the inherited environment to only allowlisted variables.
//