diff --git a/README.md b/README.md index 19ea68e2..ef024b54 100644 --- a/README.md +++ b/README.md @@ -122,6 +122,9 @@ response = client.call_tool( stdio_transport.close ``` +Stdio servers inherit only a small base environment by default, including `GEM_HOME` and `GEM_PATH`. +Pass extra variables through `env:`, or set `inherit_env: true` to restore full inheritance. + The same client can connect to Streamable HTTP servers with `MCP::Client::HTTP`; see [Client Transports](https://ruby.sdk.modelcontextprotocol.io/client/transports/). diff --git a/docs/_client/transports.md b/docs/_client/transports.md index 7f34e202..5f460c91 100644 --- a/docs/_client/transports.md +++ b/docs/_client/transports.md @@ -25,10 +25,22 @@ Use the `MCP::Client::Stdio` transport to interact with MCP servers running as s |---|---|---| | `command:` | Yes | The command to spawn the server process (e.g., `"ruby"`, `"bundle"`, `"npx"`). | | `args:` | No | An array of arguments passed to the command. Defaults to `[]`. | -| `env:` | No | A hash of environment variables to set for the server process. Defaults to `nil`. | +| `env:` | No | Extra environment variables for the server process, merged over the inherited base environment. Values override inherited variables; a `nil` value removes a variable. Defaults to `nil`. | +| `inherit_env:` | No | Inherit the full parent environment instead of the default allow-list. Defaults to `false`. | | `read_timeout:` | No | Timeout in seconds for waiting for a server response. Defaults to `nil` (no timeout). | | `max_line_bytes:` | No | Maximum byte length of a single newline-delimited response frame. A frame that reaches this limit without a newline is rejected as a transport error, preventing unbounded memory growth from a server that never emits a newline. Defaults to `4 * 1024 * 1024` (4 MiB). | +By default, the server inherits only these variables that are present in the parent environment: + +- Unix: `HOME`, `LOGNAME`, `PATH`, `SHELL`, `TERM`, `USER`, `GEM_HOME`, `GEM_PATH`. +- Windows: `APPDATA`, `HOMEDRIVE`, `HOMEPATH`, `LOCALAPPDATA`, `PATH`, `PATHEXT`, + `PROCESSOR_ARCHITECTURE`, `SYSTEMDRIVE`, `SYSTEMROOT`, `TEMP`, `USERNAME`, `USERPROFILE`, + `GEM_HOME`, `GEM_PATH`. + +**Migration:** Previous versions inherited the full parent environment. Pass required extra variables +through `env:`, or set `inherit_env: true` to restore that behavior. Full inheritance also passes +parent credentials and unrelated variables to the server. + Example usage: ```ruby diff --git a/lib/mcp/client/stdio.rb b/lib/mcp/client/stdio.rb index e2e756d3..b3122860 100644 --- a/lib/mcp/client/stdio.rb +++ b/lib/mcp/client/stdio.rb @@ -34,9 +34,20 @@ class Stdio # Matches the C# SDK's `DiscoverProbeTimeout` default. DEFAULT_DISCOVER_PROBE_TIMEOUT = 5 + # Inherited Windows environment, including RubyGems paths. + WINDOWS_INHERITED_ENV_VARS = %w[ + APPDATA HOMEDRIVE HOMEPATH LOCALAPPDATA PATH PATHEXT PROCESSOR_ARCHITECTURE + SYSTEMDRIVE SYSTEMROOT TEMP USERNAME USERPROFILE GEM_HOME GEM_PATH + ].freeze + + # Inherited Unix environment, including RubyGems paths. + UNIX_INHERITED_ENV_VARS = %w[HOME LOGNAME PATH SHELL TERM USER GEM_HOME GEM_PATH].freeze + + DEFAULT_INHERITED_ENV_VARS = Gem.win_platform? ? WINDOWS_INHERITED_ENV_VARS : UNIX_INHERITED_ENV_VARS + attr_reader :command, :args, :env, :server_info - def initialize(command:, args: [], env: nil, read_timeout: nil, max_line_bytes: MAX_LINE_BYTES) + def initialize(command:, args: [], env: nil, inherit_env: false, read_timeout: nil, max_line_bytes: MAX_LINE_BYTES) # Reject `nil` or non-positive values: `IO#gets("\n", nil)` and a negative # limit read without an upper bound, which would silently disable the # protection this option exists to provide. @@ -47,6 +58,7 @@ def initialize(command:, args: [], env: nil, read_timeout: nil, max_line_bytes: @command = command @args = args @env = env + @inherit_env = inherit_env @read_timeout = read_timeout @max_line_bytes = max_line_bytes @stdin = nil @@ -166,8 +178,8 @@ def send_notification(notification:) def start raise "MCP::Client::Stdio already started" if @started - spawn_env = @env || {} - @stdin, @stdout, @stderr, @wait_thread = Open3.popen3(spawn_env, @command, *@args) + spawn_env = DEFAULT_INHERITED_ENV_VARS.to_h { |key| [key, ENV[key]] }.compact.merge(@env || {}) + @stdin, @stdout, @stderr, @wait_thread = Open3.popen3(spawn_env, @command, *@args, unsetenv_others: !@inherit_env) @stdout.set_encoding("UTF-8") @stdin.set_encoding("UTF-8") diff --git a/test/mcp/client/stdio_environment_test.rb b/test/mcp/client/stdio_environment_test.rb new file mode 100644 index 00000000..359bdbca --- /dev/null +++ b/test/mcp/client/stdio_environment_test.rb @@ -0,0 +1,138 @@ +# frozen_string_literal: true + +require "test_helper" +require "json" +require "rbconfig" +require "mcp/client/stdio" + +module MCP + class Client + class StdioEnvironmentTest < Minitest::Test + def test_default_environment_does_not_inherit_unrelated_variables + with_environment("MCP_TEST_HOST_ONLY" => "host-value") do + refute(server_environment.key?("MCP_TEST_HOST_ONLY")) + end + end + + def test_empty_env_does_not_inherit_unrelated_variables + with_environment("MCP_TEST_HOST_ONLY" => "host-value") do + refute(server_environment(env: {}).key?("MCP_TEST_HOST_ONLY")) + end + end + + def test_explicit_env_does_not_inherit_unrelated_variables + with_environment("MCP_TEST_HOST_ONLY" => "host-value") do + environment = server_environment(env: { "MCP_TEST_SERVER_SETTING" => "server-value" }) + + assert_equal("server-value", environment["MCP_TEST_SERVER_SETTING"]) + refute(environment.key?("MCP_TEST_HOST_ONLY")) + end + end + + def test_default_environment_preserves_path_and_home + home_variable = Gem.win_platform? ? "USERPROFILE" : "HOME" + with_environment(home_variable => "server-home") do + environment = server_environment + + assert_equal("server-home", environment[home_variable]) + assert_equal(ENV.fetch("PATH"), environment["PATH"]) + end + end + + def test_default_environment_preserves_ruby_configuration + variables = { + "GEM_HOME" => Gem.dir, + "GEM_PATH" => Gem.path.join(File::PATH_SEPARATOR), + } + with_environment(variables) do + assert_equal(variables, server_environment.slice(*variables.keys)) + end + end + + def test_default_environment_does_not_inherit_bundle_gemfile + with_environment("BUNDLE_GEMFILE" => File.expand_path("client/Gemfile")) do + refute(server_environment.key?("BUNDLE_GEMFILE")) + end + end + + def test_explicit_env_can_select_bundle_gemfile + gemfile = File.expand_path("server/Gemfile") + with_environment("BUNDLE_GEMFILE" => File.expand_path("client/Gemfile")) do + environment = server_environment(env: { "BUNDLE_GEMFILE" => gemfile }) + + assert_equal(gemfile, environment["BUNDLE_GEMFILE"]) + end + end + + def test_default_environment_preserves_case_insensitive_variable_lookups + # Windows retains mixed-case names in ENV.to_h but accepts uppercase ENV lookups. + ENV.stubs(:to_h).returns("Path" => "server-path") + ENV.stubs(:[]).returns(nil) + ENV.stubs(:[]).with("PATH").returns("server-path") + + assert_equal("server-path", server_environment["PATH"]) + end + + def test_explicit_env_overrides_default_variables + environment = server_environment(env: { "PATH" => "server-path" }) + + assert_equal("server-path", environment["PATH"]) + end + + def test_explicit_nil_removes_a_default_variable + with_environment("PATH" => "parent-path") do + refute(server_environment(env: { "PATH" => nil }).key?("PATH")) + end + end + + def test_missing_default_variable_is_not_added + with_environment("TERM" => nil) do + refute(server_environment.key?("TERM")) + end + end + + def test_inherit_env_restores_full_environment_inheritance + with_environment("MCP_TEST_HOST_ONLY" => "host-value") do + environment = server_environment(inherit_env: true) + + assert_equal("host-value", environment["MCP_TEST_HOST_ONLY"]) + end + end + + def test_explicit_env_overrides_and_removes_variables_when_inheriting + with_environment("MCP_TEST_HOST_ONLY" => "host-value", "MCP_TEST_REMOVE" => "remove-value") do + environment = server_environment( + inherit_env: true, + env: { "MCP_TEST_HOST_ONLY" => "server-value", "MCP_TEST_REMOVE" => nil }, + ) + + assert_equal("server-value", environment["MCP_TEST_HOST_ONLY"]) + refute(environment.key?("MCP_TEST_REMOVE")) + end + end + + private + + def server_environment(**options) + transport = Stdio.new( + command: RbConfig.ruby, + args: ["-e", "require 'json'; puts JSON.generate(ENV.to_h)"], + **options, + ) + transport.start + output = Timeout.timeout(5) { transport.instance_variable_get(:@stdout).read } + JSON.parse(output) + ensure + transport&.close + end + + def with_environment(variables) + previous_values = variables.to_h { |key, _| [key, ENV[key]] } + variables.each { |key, value| ENV[key] = value } + yield + ensure + previous_values.each { |key, value| ENV[key] = value } + end + end + end +end diff --git a/test/mcp/client/stdio_test.rb b/test/mcp/client/stdio_test.rb index 7a04a6ee..907ecab9 100644 --- a/test/mcp/client/stdio_test.rb +++ b/test/mcp/client/stdio_test.rb @@ -679,7 +679,8 @@ def test_multiple_send_requests_do_not_reinitialize def test_env_is_passed_to_process transport = Stdio.new(command: "ruby", args: ["server.rb"], env: { "FOO" => "bar" }) - Open3.expects(:popen3).with({ "FOO" => "bar" }, "ruby", "server.rb").returns( + spawn_env = Stdio::DEFAULT_INHERITED_ENV_VARS.to_h { |key| [key, ENV[key]] }.compact.merge("FOO" => "bar") + Open3.expects(:popen3).with(spawn_env, "ruby", "server.rb", unsetenv_others: true).returns( [StringIO.new, StringIO.new, StringIO.new, mock_wait_thread], )