Skip to content

Inherit environment variables deemed safe by default #27

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 7, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 36 additions & 3 deletions mcp_python/client/stdio.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import os
import sys
from contextlib import asynccontextmanager

Expand All @@ -9,6 +10,36 @@

from mcp_python.types import JSONRPCMessage

# Environment variables to inherit by default
DEFAULT_INHERITED_ENV_VARS = (
["APPDATA", "HOMEDRIVE", "HOMEPATH", "LOCALAPPDATA", "PATH",
"PROCESSOR_ARCHITECTURE", "SYSTEMDRIVE", "SYSTEMROOT", "TEMP",
"USERNAME", "USERPROFILE"]
if sys.platform == "win32"
else ["HOME", "LOGNAME", "PATH", "SHELL", "TERM", "USER"]
)


def get_default_environment() -> dict[str, str]:
"""
Returns a default environment object including only environment variables deemed
safe to inherit.
"""
env: dict[str, str] = {}

for key in DEFAULT_INHERITED_ENV_VARS:
value = os.environ.get(key)
if value is None:
continue

if value.startswith("()"):
# Skip functions, which are a security risk
continue
Comment on lines +35 to +37
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are more ways of defining functions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is copied from the behavior of sudo


env[key] = value

return env


class StdioServerParameters(BaseModel):
command: str
Expand All @@ -17,11 +48,11 @@ class StdioServerParameters(BaseModel):
args: list[str] = Field(default_factory=list)
"""Command line arguments to pass to the executable."""

env: dict[str, str] = Field(default_factory=dict)
env: dict[str, str] | None = None
"""
The environment to use when spawning the process.

The environment is NOT inherited from the parent process by default.
If not specified, the result of get_default_environment() will be used.
"""


Expand All @@ -41,7 +72,9 @@ async def stdio_client(server: StdioServerParameters):
write_stream, write_stream_reader = anyio.create_memory_object_stream(0)

process = await anyio.open_process(
[server.command, *server.args], env=server.env, stderr=sys.stderr
[server.command, *server.args],
env=server.env if server.env is not None else get_default_environment(),
stderr=sys.stderr
)

async def stdout_reader():
Expand Down