diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index 587827251..e2bff3658 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -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 diff --git a/pkg/tools/shell/env.go b/pkg/tools/shell/env.go index 3abd5c499..a716078e1 100644 --- a/pkg/tools/shell/env.go +++ b/pkg/tools/shell/env.go @@ -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. //