From b4dd28b5eca8eec26667a370e6ade813c43c8f9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomas=20Daba=C5=A1inskas?= Date: Sun, 3 May 2026 20:00:52 +0300 Subject: [PATCH] fix(tools): prevent environment variable race in script shell tool The script shell tool's `execute()` method was aliasing the shared `t.env` slice directly into `cmd.Env`, then appending per-call parameters. When the slice had spare capacity, appending would mutate the shared backing array, causing concurrent tool invocations to race and leak parameters into each other's environments. Now create a per-call copy with exact capacity equal to the final size, ensuring each subprocess receives only its own parameters without affecting concurrent executions. fix(tools): preserve nil-env semantics in script_shell so child inherits parent env When `t.env` is nil (no toolset env wired), the prior fix changed `cmd.Env=nil` (Go inherits parent env) to a non-nil empty slice, stripping PATH/HOME/everything from the subprocess. Expand nil to `os.Environ()` before the per-call clone so the inherit-parent semantics are preserved. Addresses Copilot review feedback on the prior fix in this branch. --- pkg/tools/builtin/script_shell.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/pkg/tools/builtin/script_shell.go b/pkg/tools/builtin/script_shell.go index 4d3e148bf..13c69f9f8 100644 --- a/pkg/tools/builtin/script_shell.go +++ b/pkg/tools/builtin/script_shell.go @@ -142,12 +142,22 @@ func (t *ScriptShellTool) execute(ctx context.Context, toolConfig *latest.Script cmd := exec.CommandContext(ctx, shell, append(argsPrefix, toolConfig.Cmd)...) cmd.Dir = toolConfig.WorkingDir - cmd.Env = t.env + // Per-call clone: appending onto t.env would mutate the shared + // backing array under concurrent calls. Expand nil to os.Environ() + // so a nil t.env still inherits the parent env (a non-nil empty + // slice would strip it). + base := t.env + if base == nil { + base = os.Environ() + } + envCopy := make([]string, len(base), len(base)+len(params)) + copy(envCopy, base) for key, value := range params { if value != nil { - cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%v", key, value)) + envCopy = append(envCopy, fmt.Sprintf("%s=%v", key, value)) } } + cmd.Env = envCopy output, err := cmd.CombinedOutput() if err != nil {