Skip to content

Commit e197191

Browse files
authored
Merge pull request #665 from atesgoral/fix/stdio-client-environment
fix: restrict the stdio client server environment
2 parents 5bd9b7a + 25d3f95 commit e197191

5 files changed

Lines changed: 171 additions & 5 deletions

File tree

‎README.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,9 @@ response = client.call_tool(
122122
stdio_transport.close
123123
```
124124

125+
Stdio servers inherit only a small base environment by default, including `GEM_HOME` and `GEM_PATH`.
126+
Pass extra variables through `env:`, or set `inherit_env: true` to restore full inheritance.
127+
125128
The same client can connect to Streamable HTTP servers with `MCP::Client::HTTP`;
126129
see [Client Transports](https://ruby.sdk.modelcontextprotocol.io/client/transports/).
127130

‎docs/_client/transports.md‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,22 @@ Use the `MCP::Client::Stdio` transport to interact with MCP servers running as s
2525
|---|---|---|
2626
| `command:` | Yes | The command to spawn the server process (e.g., `"ruby"`, `"bundle"`, `"npx"`). |
2727
| `args:` | No | An array of arguments passed to the command. Defaults to `[]`. |
28-
| `env:` | No | A hash of environment variables to set for the server process. Defaults to `nil`. |
28+
| `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`. |
29+
| `inherit_env:` | No | Inherit the full parent environment instead of the default allow-list. Defaults to `false`. |
2930
| `read_timeout:` | No | Timeout in seconds for waiting for a server response. Defaults to `nil` (no timeout). |
3031
| `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). |
3132

33+
By default, the server inherits only these variables that are present in the parent environment:
34+
35+
- Unix: `HOME`, `LOGNAME`, `PATH`, `SHELL`, `TERM`, `USER`, `GEM_HOME`, `GEM_PATH`.
36+
- Windows: `APPDATA`, `HOMEDRIVE`, `HOMEPATH`, `LOCALAPPDATA`, `PATH`, `PATHEXT`,
37+
`PROCESSOR_ARCHITECTURE`, `SYSTEMDRIVE`, `SYSTEMROOT`, `TEMP`, `USERNAME`, `USERPROFILE`,
38+
`GEM_HOME`, `GEM_PATH`.
39+
40+
**Migration:** Previous versions inherited the full parent environment. Pass required extra variables
41+
through `env:`, or set `inherit_env: true` to restore that behavior. Full inheritance also passes
42+
parent credentials and unrelated variables to the server.
43+
3244
Example usage:
3345

3446
```ruby

‎lib/mcp/client/stdio.rb‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,20 @@ class Stdio
3434
# Matches the C# SDK's `DiscoverProbeTimeout` default.
3535
DEFAULT_DISCOVER_PROBE_TIMEOUT = 5
3636

37+
# Inherited Windows environment, including RubyGems paths.
38+
WINDOWS_INHERITED_ENV_VARS = %w[
39+
APPDATA HOMEDRIVE HOMEPATH LOCALAPPDATA PATH PATHEXT PROCESSOR_ARCHITECTURE
40+
SYSTEMDRIVE SYSTEMROOT TEMP USERNAME USERPROFILE GEM_HOME GEM_PATH
41+
].freeze
42+
43+
# Inherited Unix environment, including RubyGems paths.
44+
UNIX_INHERITED_ENV_VARS = %w[HOME LOGNAME PATH SHELL TERM USER GEM_HOME GEM_PATH].freeze
45+
46+
DEFAULT_INHERITED_ENV_VARS = Gem.win_platform? ? WINDOWS_INHERITED_ENV_VARS : UNIX_INHERITED_ENV_VARS
47+
3748
attr_reader :command, :args, :env, :server_info
3849

39-
def initialize(command:, args: [], env: nil, read_timeout: nil, max_line_bytes: MAX_LINE_BYTES)
50+
def initialize(command:, args: [], env: nil, inherit_env: false, read_timeout: nil, max_line_bytes: MAX_LINE_BYTES)
4051
# Reject `nil` or non-positive values: `IO#gets("\n", nil)` and a negative
4152
# limit read without an upper bound, which would silently disable the
4253
# protection this option exists to provide.
@@ -47,6 +58,7 @@ def initialize(command:, args: [], env: nil, read_timeout: nil, max_line_bytes:
4758
@command = command
4859
@args = args
4960
@env = env
61+
@inherit_env = inherit_env
5062
@read_timeout = read_timeout
5163
@max_line_bytes = max_line_bytes
5264
@stdin = nil
@@ -166,8 +178,8 @@ def send_notification(notification:)
166178
def start
167179
raise "MCP::Client::Stdio already started" if @started
168180

169-
spawn_env = @env || {}
170-
@stdin, @stdout, @stderr, @wait_thread = Open3.popen3(spawn_env, @command, *@args)
181+
spawn_env = DEFAULT_INHERITED_ENV_VARS.to_h { |key| [key, ENV[key]] }.compact.merge(@env || {})
182+
@stdin, @stdout, @stderr, @wait_thread = Open3.popen3(spawn_env, @command, *@args, unsetenv_others: !@inherit_env)
171183
@stdout.set_encoding("UTF-8")
172184
@stdin.set_encoding("UTF-8")
173185

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
# frozen_string_literal: true
2+
3+
require "test_helper"
4+
require "json"
5+
require "rbconfig"
6+
require "mcp/client/stdio"
7+
8+
module MCP
9+
class Client
10+
class StdioEnvironmentTest < Minitest::Test
11+
def test_default_environment_does_not_inherit_unrelated_variables
12+
with_environment("MCP_TEST_HOST_ONLY" => "host-value") do
13+
refute(server_environment.key?("MCP_TEST_HOST_ONLY"))
14+
end
15+
end
16+
17+
def test_empty_env_does_not_inherit_unrelated_variables
18+
with_environment("MCP_TEST_HOST_ONLY" => "host-value") do
19+
refute(server_environment(env: {}).key?("MCP_TEST_HOST_ONLY"))
20+
end
21+
end
22+
23+
def test_explicit_env_does_not_inherit_unrelated_variables
24+
with_environment("MCP_TEST_HOST_ONLY" => "host-value") do
25+
environment = server_environment(env: { "MCP_TEST_SERVER_SETTING" => "server-value" })
26+
27+
assert_equal("server-value", environment["MCP_TEST_SERVER_SETTING"])
28+
refute(environment.key?("MCP_TEST_HOST_ONLY"))
29+
end
30+
end
31+
32+
def test_default_environment_preserves_path_and_home
33+
home_variable = Gem.win_platform? ? "USERPROFILE" : "HOME"
34+
with_environment(home_variable => "server-home") do
35+
environment = server_environment
36+
37+
assert_equal("server-home", environment[home_variable])
38+
assert_equal(ENV.fetch("PATH"), environment["PATH"])
39+
end
40+
end
41+
42+
def test_default_environment_preserves_ruby_configuration
43+
variables = {
44+
"GEM_HOME" => Gem.dir,
45+
"GEM_PATH" => Gem.path.join(File::PATH_SEPARATOR),
46+
}
47+
with_environment(variables) do
48+
assert_equal(variables, server_environment.slice(*variables.keys))
49+
end
50+
end
51+
52+
def test_default_environment_does_not_inherit_bundle_gemfile
53+
with_environment("BUNDLE_GEMFILE" => File.expand_path("client/Gemfile")) do
54+
refute(server_environment.key?("BUNDLE_GEMFILE"))
55+
end
56+
end
57+
58+
def test_explicit_env_can_select_bundle_gemfile
59+
gemfile = File.expand_path("server/Gemfile")
60+
with_environment("BUNDLE_GEMFILE" => File.expand_path("client/Gemfile")) do
61+
environment = server_environment(env: { "BUNDLE_GEMFILE" => gemfile })
62+
63+
assert_equal(gemfile, environment["BUNDLE_GEMFILE"])
64+
end
65+
end
66+
67+
def test_default_environment_preserves_case_insensitive_variable_lookups
68+
# Windows retains mixed-case names in ENV.to_h but accepts uppercase ENV lookups.
69+
ENV.stubs(:to_h).returns("Path" => "server-path")
70+
ENV.stubs(:[]).returns(nil)
71+
ENV.stubs(:[]).with("PATH").returns("server-path")
72+
73+
assert_equal("server-path", server_environment["PATH"])
74+
end
75+
76+
def test_explicit_env_overrides_default_variables
77+
environment = server_environment(env: { "PATH" => "server-path" })
78+
79+
assert_equal("server-path", environment["PATH"])
80+
end
81+
82+
def test_explicit_nil_removes_a_default_variable
83+
with_environment("PATH" => "parent-path") do
84+
refute(server_environment(env: { "PATH" => nil }).key?("PATH"))
85+
end
86+
end
87+
88+
def test_missing_default_variable_is_not_added
89+
with_environment("TERM" => nil) do
90+
refute(server_environment.key?("TERM"))
91+
end
92+
end
93+
94+
def test_inherit_env_restores_full_environment_inheritance
95+
with_environment("MCP_TEST_HOST_ONLY" => "host-value") do
96+
environment = server_environment(inherit_env: true)
97+
98+
assert_equal("host-value", environment["MCP_TEST_HOST_ONLY"])
99+
end
100+
end
101+
102+
def test_explicit_env_overrides_and_removes_variables_when_inheriting
103+
with_environment("MCP_TEST_HOST_ONLY" => "host-value", "MCP_TEST_REMOVE" => "remove-value") do
104+
environment = server_environment(
105+
inherit_env: true,
106+
env: { "MCP_TEST_HOST_ONLY" => "server-value", "MCP_TEST_REMOVE" => nil },
107+
)
108+
109+
assert_equal("server-value", environment["MCP_TEST_HOST_ONLY"])
110+
refute(environment.key?("MCP_TEST_REMOVE"))
111+
end
112+
end
113+
114+
private
115+
116+
def server_environment(**options)
117+
transport = Stdio.new(
118+
command: RbConfig.ruby,
119+
args: ["-e", "require 'json'; puts JSON.generate(ENV.to_h)"],
120+
**options,
121+
)
122+
transport.start
123+
output = Timeout.timeout(5) { transport.instance_variable_get(:@stdout).read }
124+
JSON.parse(output)
125+
ensure
126+
transport&.close
127+
end
128+
129+
def with_environment(variables)
130+
previous_values = variables.to_h { |key, _| [key, ENV[key]] }
131+
variables.each { |key, value| ENV[key] = value }
132+
yield
133+
ensure
134+
previous_values.each { |key, value| ENV[key] = value }
135+
end
136+
end
137+
end
138+
end

‎test/mcp/client/stdio_test.rb‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -679,7 +679,8 @@ def test_multiple_send_requests_do_not_reinitialize
679679
def test_env_is_passed_to_process
680680
transport = Stdio.new(command: "ruby", args: ["server.rb"], env: { "FOO" => "bar" })
681681

682-
Open3.expects(:popen3).with({ "FOO" => "bar" }, "ruby", "server.rb").returns(
682+
spawn_env = Stdio::DEFAULT_INHERITED_ENV_VARS.to_h { |key| [key, ENV[key]] }.compact.merge("FOO" => "bar")
683+
Open3.expects(:popen3).with(spawn_env, "ruby", "server.rb", unsetenv_others: true).returns(
683684
[StringIO.new, StringIO.new, StringIO.new, mock_wait_thread],
684685
)
685686

0 commit comments

Comments
 (0)