Refactor env handling: extract MapToEnvSlice, clean up unused code
- Remove unused BuildSanitizedEnv function
- Extract MapToEnvSlice helper from MergeEnvVars
- MergeEnvVars now returns map, caller converts to []string
- Fix lint issues (gci, gofumpt, golines, whitespace)
💘 Generated with Crush
This commit is contained in:
parent
369edf30af
commit
4b11ef32fe
3 changed files with 50 additions and 127 deletions
|
|
@ -263,7 +263,7 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult
|
|||
|
||||
// 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)
|
||||
cmd.Env = shell.MapToEnvSlice(shell.MergeEnvVars(t.cachedEnv, execTimeEnv, extraEnv))
|
||||
|
||||
if cwd != "" {
|
||||
cmd.Dir = cwd
|
||||
|
|
|
|||
|
|
@ -52,12 +52,6 @@ var DefaultEnvAllowlist = map[string]bool{
|
|||
"LC_COLLATE": true,
|
||||
}
|
||||
|
||||
// defaultEnvAllowPrefixes are env var prefixes that are always allowed.
|
||||
// Currently empty - all allowed vars are explicit in DefaultEnvAllowlist.
|
||||
var defaultEnvAllowPrefixes = []string{
|
||||
// Currently empty - all allowed vars are explicit
|
||||
}
|
||||
|
||||
// LLMBlocklist is the set of environment variable names that the LLM
|
||||
// cannot override, even if passed via the env parameter. These vars
|
||||
// control fundamental process behavior and could be exploited.
|
||||
|
|
@ -154,11 +148,11 @@ func isBlocked(key string) bool {
|
|||
return false
|
||||
}
|
||||
|
||||
// MergeEnvVars merges multiple env sources into a final []string for exec.Cmd.Env.
|
||||
// MergeEnvVars merges multiple env sources into a map.
|
||||
// baseEnv is the cached map from AllowedEnv.
|
||||
// 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 {
|
||||
func MergeEnvVars(baseEnv map[string]string, envSet, extraEnv map[string]string) map[string]string {
|
||||
vars := make(map[string]string, len(baseEnv)+len(envSet)+len(extraEnv))
|
||||
|
||||
// Start with base env (already filtered)
|
||||
|
|
@ -183,74 +177,12 @@ func MergeEnvVars(baseEnv map[string]string, envSet, extraEnv map[string]string)
|
|||
}
|
||||
}
|
||||
|
||||
// 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
|
||||
return vars
|
||||
}
|
||||
|
||||
// BuildSanitizedEnv constructs a sanitized environment []string suitable for
|
||||
// exec.Cmd.Env. It filters the inherited environment to only allowlisted variables.
|
||||
//
|
||||
// baseEnv is the inherited environment (e.g., from os.Environ() or cached).
|
||||
// If nil, os.Environ() will be used for backwards compatibility.
|
||||
// extraAllowlist adds additional variable names to the default allowlist.
|
||||
// envSet provides explicit key=value pairs from config (override inherited).
|
||||
// extraEnv provides additional key=value pairs from tool call (merged with envSet).
|
||||
func BuildSanitizedEnv(baseEnv []string, extraAllowlist []string, envSet, extraEnv map[string]string) []string {
|
||||
|
||||
// Use provided env or fall back to os.Environ
|
||||
inherited := baseEnv
|
||||
if inherited == nil {
|
||||
inherited = 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, len(allowed)+len(envSet)+len(extraEnv))
|
||||
|
||||
for _, entry := range inherited {
|
||||
k, v, ok := strings.Cut(entry, "=")
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
norm := envKey(k)
|
||||
if allowed[norm] || isAllowedPrefix(norm) {
|
||||
vars[norm] = v
|
||||
}
|
||||
}
|
||||
|
||||
if envSet != nil {
|
||||
for k, v := range envSet {
|
||||
vars[envKey(k)] = v
|
||||
}
|
||||
}
|
||||
|
||||
// Merge extraEnv (tool call) - highest priority
|
||||
// Filter against LLM blocklist to prevent override of sensitive vars
|
||||
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
|
||||
// MapToEnvSlice converts a map of environment variables to a []string
|
||||
// in the format "KEY=value" suitable for exec.Cmd.Env.
|
||||
func MapToEnvSlice(vars map[string]string) []string {
|
||||
result := make([]string, 0, len(vars))
|
||||
for k, v := range vars {
|
||||
result = append(result, k+"="+v)
|
||||
|
|
@ -268,15 +200,6 @@ func envKey(k string) string {
|
|||
return k
|
||||
}
|
||||
|
||||
func isAllowedPrefix(name string) bool {
|
||||
for _, prefix := range defaultEnvAllowPrefixes {
|
||||
if strings.HasPrefix(name, prefix) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// WithPicoclawEnvVars ensures PICOCLAW_* vars are set in envSet.
|
||||
// These are needed for child processes to locate config, workspace, etc.
|
||||
func WithPicoclawEnvVars(envSet map[string]string, workspace string) map[string]string {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue