Skip to content

Commit ead7eda

Browse files
committed
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.
1 parent 93ba85c commit ead7eda

1 file changed

Lines changed: 12 additions & 2 deletions

File tree

pkg/tools/builtin/script_shell.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,12 +142,22 @@ func (t *ScriptShellTool) execute(ctx context.Context, toolConfig *latest.Script
142142

143143
cmd := exec.CommandContext(ctx, shell, append(argsPrefix, toolConfig.Cmd)...)
144144
cmd.Dir = toolConfig.WorkingDir
145-
cmd.Env = t.env
145+
// Per-call clone: appending onto t.env would mutate the shared
146+
// backing array under concurrent calls. Expand nil to os.Environ()
147+
// so a nil t.env still inherits the parent env (a non-nil empty
148+
// slice would strip it).
149+
base := t.env
150+
if base == nil {
151+
base = os.Environ()
152+
}
153+
envCopy := make([]string, len(base), len(base)+len(params))
154+
copy(envCopy, base)
146155
for key, value := range params {
147156
if value != nil {
148-
cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%v", key, value))
157+
envCopy = append(envCopy, fmt.Sprintf("%s=%v", key, value))
149158
}
150159
}
160+
cmd.Env = envCopy
151161

152162
output, err := cmd.CombinedOutput()
153163
if err != nil {

0 commit comments

Comments
 (0)