Add InheritEnvironmentVariables to StdioClientTransportOptions#1563
Open
Add InheritEnvironmentVariables to StdioClientTransportOptions#1563
InheritEnvironmentVariables to StdioClientTransportOptions#1563Conversation
Introduces a new bool property InheritEnvironmentVariables (default: true) on StdioClientTransportOptions that controls whether the child server process inherits the current process's environment variables. When false, startInfo.Environment.Clear() is called before any EnvironmentVariables entries are applied, giving the child process a completely clean environment. This allows callers to prevent unintentional leakage of credentials, tokens, and proxy settings into third-party or untrusted MCP server processes. The new property is backward-compatible: the default (true) preserves the existing behavior of inheriting all parent env vars and then layering EnvironmentVariables on top. Three tests are added to StdioClientTransportTests: - InheritEnvironmentVariables_DefaultTrue_ChildSeesParentEnvVars - InheritEnvironmentVariables_False_ChildDoesNotSeeParentEnvVars - InheritEnvironmentVariables_False_WithExplicitVars_ChildSeesOnlyExplicitVars Documentation in transports.md is updated with: - New property in the stdio options table - A dedicated 'Environment variable inheritance' section with a code sample and a WARNING callout explaining both the security risk of inheriting (credential leakage) and the compatibility risk of disabling (PATH, HOME, DOTNET_ROOT etc. required by many tools). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Instead of calling Environment.SetEnvironmentVariable (which is process-global
and can cause race conditions with parallel tests), rely on PATH which is always
present in a real process environment as the 'known parent variable'.
For the no-inherit tests, use absolute shell paths (/bin/sh on Unix, full
cmd.exe path on Windows from SystemRoot) so the child can launch even with
a completely empty environment — no PATH needed.
Test 1: default inheritance → child sees PATH (no parent env mutation)
Test 2: InheritEnvironmentVariables=false → child does NOT see PATH
Test 3: InheritEnvironmentVariables=false + explicit var → PATH absent,
explicit MCP_STDIO_TEST_EXPLICIT_VAR is present
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…te paths Use PATH (passed explicitly when InheritEnvironmentVariables=false) to find cmd/sh rather than hardcoded absolute paths. Verify inheritance by checking variables that are always present in a real process but deliberately omitted from the explicit EnvironmentVariables dict: HOME (Unix) and USERNAME (Windows). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Both QuickstartClient and ChatWithTools now disable environment variable inheritance and forward only the variables their server processes require: PATH, HOME/USERPROFILE, APPDATA, LOCALAPPDATA, TEMP/TMPDIR for Node/npm; plus DOTNET_ROOT and NUGET_PACKAGES for the dotnet case. This prevents credentials, tokens, and other sensitive parent-process variables from leaking into third-party MCP server processes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…se test Both DefaultTrue and False tests now check HOME (Unix) / USERNAME (Windows) so they form a symmetric pair asserting opposite outcomes on the same signal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Returns a curated allowlist of env vars (PATH, HOME, system dirs, etc.) safe to forward to child processes, aligned with the TypeScript and Python MCP SDK defaults. Replaces the duplicated MinimalEnvironment() helpers in the samples and updates transports.md to demonstrate the new API. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ocs and XML Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
OrdinalIgnoreCase on Windows to match ProcessStartInfo.Environment behavior, Ordinal on Unix where env var names are case-sensitive. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Do we have an option to exclude specific environment variables? For example, could we inherit all variables via |
tarekgh
reviewed
May 6, 2026
| | `Name` | Optional transport identifier for logging | | ||
|
|
||
| #### Environment variable inheritance | ||
|
|
There was a problem hiding this comment.
Do we need to mention anything regarding Windows shell wrapping?
tarekgh
reviewed
May 6, 2026
| messages.AddMessages(updates); | ||
| } No newline at end of file | ||
| } | ||
|
|
tarekgh
reviewed
May 6, 2026
| // Returns the safe default environment variables plus extras needed by 'dotnet run'. | ||
| // Omitting variables the server doesn't need prevents unintentional leakage of | ||
| // credentials or other sensitive values present in the parent process. | ||
| static Dictionary<string, string?> MinimalDotNetEnvironment() |
There was a problem hiding this comment.
nit: call it GetMinimalDotNetEnvironment?
tarekgh
reviewed
May 6, 2026
| @@ -150,6 +150,111 @@ public async Task EscapesCliArgumentsCorrectly(string? cliArgumentValue) | |||
| Assert.Equal(cliArgumentValue ?? "", content.Text); | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
As GetDefaultEnvironmentVariables() is public method, should we have dedicated test for it? it can test:
- Shell function filtering (value.StartsWith("()"))
- Platform comparer (OrdinalIgnoreCase on Windows vs Ordinal on Unix)
- Returns a fresh dictionary each call
- Only includes allowlisted vars that exist, excludes others
tarekgh
approved these changes
May 6, 2026
tarekgh
left a comment
There was a problem hiding this comment.
Added minor comments and question. LGTM, otherwise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Introduces a new
bool InheritEnvironmentVariables { get; set; } = trueproperty onStdioClientTransportOptionsthat controls whether the child server process inherits the current process's environment variables.Motivation
Previously,
EnvironmentVariablesonly augmented/overwrote inherited vars — there was no way to start the server with a clean environment. Retconning the dictionary to mean "only these vars" would be breaking.Behavior
InheritEnvironmentVariablesEnvironmentVariablestrue(default)nulltrue(default){ "X": "y" }X=yon topfalsenullfalse{ "PATH": "...", "API_KEY": "..." }When
false,startInfo.Environment.Clear()is called before anyEnvironmentVariablesentries are applied.Security / Compatibility note
The docs and XML comments explain both sides:
AWS_SECRET_ACCESS_KEY,GITHUB_TOKEN,OPENAI_API_KEYautomatically flow into the child process.PATH,HOME,DOTNET_ROOT,LD_LIBRARY_PATH,JAVA_HOME, proxy vars etc. are required by many tools — forgetting them causes the process to fail to start.Changes
StdioClientTransportOptions.cs— newInheritEnvironmentVariablesproperty with comprehensive XML docsStdioClientTransport.cs— callsstartInfo.Environment.Clear()when!InheritEnvironmentVariablesStdioClientTransportTests.cs— 3 new tests verifying inherit, block, and explicit-only scenariostransports.md— new property in the options table + "Environment variable inheritance" section with code sample and[!WARNING]callout