Skip to content

Commit 360c65d

Browse files
committed
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 089c606 commit 360c65d

1 file changed

Lines changed: 10 additions & 8 deletions

File tree

pkg/tools/builtin/script_shell.go

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -142,14 +142,16 @@ 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-
// `t.env` is shared across every concurrent invocation of execute().
146-
// Aliasing it into cmd.Env and then appending the per-call params
147-
// would mutate the shared backing array whenever cap > len, racing
148-
// with sibling tool calls and silently leaking one call's params
149-
// into another's environment. Materialise a per-call slice with
150-
// exact capacity so each subprocess sees only its own params.
151-
envCopy := make([]string, len(t.env), len(t.env)+len(params))
152-
copy(envCopy, 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)
153155
for key, value := range params {
154156
if value != nil {
155157
envCopy = append(envCopy, fmt.Sprintf("%s=%v", key, value))

0 commit comments

Comments
 (0)