Skip to content

fix(tools): prevent environment variable race in script shell tool#2616

Merged
dgageot merged 1 commit intodocker:mainfrom
cogvel:fix/script-shell-env-race
May 4, 2026
Merged

fix(tools): prevent environment variable race in script shell tool#2616
dgageot merged 1 commit intodocker:mainfrom
cogvel:fix/script-shell-env-race

Conversation

@tdabasinskas
Copy link
Copy Markdown
Contributor

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.

@tdabasinskas tdabasinskas requested a review from a team as a code owner May 3, 2026 17:01
@tdabasinskas tdabasinskas marked this pull request as draft May 4, 2026 07:53
@tdabasinskas tdabasinskas force-pushed the fix/script-shell-env-race branch 3 times, most recently from 360c65d to ead7eda Compare May 4, 2026 08:19
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.
@tdabasinskas tdabasinskas force-pushed the fix/script-shell-env-race branch from ead7eda to b4dd28b Compare May 4, 2026 08:20
@tdabasinskas tdabasinskas marked this pull request as ready for review May 4, 2026 08:24
@dgageot dgageot merged commit 1cc4fa9 into docker:main May 4, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants