Skip to content
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

MCP Client Environment Variables in StdioServerParameters #99

Closed
AshuJoshi opened this issue Dec 11, 2024 · 7 comments
Closed

MCP Client Environment Variables in StdioServerParameters #99

AshuJoshi opened this issue Dec 11, 2024 · 7 comments
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers

Comments

@AshuJoshi
Copy link

Describe the bug
Passing environment variables to the MCP Server - I am trying to test the example Weather MCP Server with both the MCP Inspector and a simple Python MCP Client, and when I try to pass the Open Weather API key using the 'env' field - I cannot it to work.

To Reproduce
Steps to reproduce the behavior:

  1. Use StdioServerParameters to configure connect with MCP server in Python Client
  2. Specify the Open Weather Key in the parameters
  3. When running the Python client - you get an error
    [Errno 2] No such file or directory: 'uv'

Here is the code snippet from the Python Client:

server_params = StdioServerParameters(
        command="uv",
        args=[
            "--directory",
            "<PATH-TO-SERVER>/weather_service",
            "run",
            "weather-service"
            ],
        # env=None
        env={
            "OPENWEATHER_API_KEY" : API_KEY
        }
    )

    # Initialize MCP client with server parameters
    async with MCPClient(server_params) as mcp_client:

If I do not pass any environment by either commenting the env field or remove it all together, then the Client is able to connect with the Server.
Are there any examples on using StdioServerParameters in Python Clients? I could not find any examples (other than env=None) in example MCP Client code.

@christopherhwood
Copy link

christopherhwood commented Dec 12, 2024

You need to make sure to include the necessary default env vars for it to find the uv program. When you set env to None then it uses these default env vars:

# 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
env[key] = value
return env

As a fast workaround, if you copy over the above code for the get_default_environment and use that as the base env dict, then update with your new env vars and pass that through to the mcp then you should see it working, something like this:

async def connect_to_server(self, server_config: ServerConfig):
    env = get_default_environment()
    env.update(server_config.env) if server_config.env else None

    server_params = StdioServerParameters(
        command=server_config.command,
        args=server_config.args,
        env=env
    )
    # rest of code here...

@dsp-ant dsp-ant added documentation Improvements or additions to documentation good first issue Good for newcomers labels Jan 7, 2025
@kzmszk
Copy link

kzmszk commented Jan 10, 2025

I believe it would be much better to apply the above code into the sdk.

https://github.com/modelcontextprotocol/python-sdk/blob/main/src/mcp/client/stdio.py#L100-L103

@BryceShakeAir
Copy link

BryceShakeAir commented Jan 12, 2025

I propose adding an optional flag append_default_env_vars to StdioServerParameters to control whether default environment variables should be merged with custom environment variables. This would maintain backward compatibility
Example:

class StdioServerParameters(BaseModel):
    append_default_env_vars: bool = Field(default=False, 
        description="When True, merges default environment variables with custom env")

@zhjch05
Copy link

zhjch05 commented Feb 3, 2025

Oh I just saw this and I found similar errors and I proposed some code in #185 too

we can have a flag like @BryceShakeAir mentioned and then merge the env vars like this

process = await anyio.open_process(
      [server.command, *server.args],
      env={
          **get_default_environment(),
          **(server.env or {})
      },
      stderr=sys.stderr,
)

@sbshah97
Copy link

sbshah97 commented Mar 1, 2025

Is this issue still open for contribution? Would love to send a documentation patch if that helps?

@ggozad
Copy link

ggozad commented Mar 15, 2025

I stumbled upon this because of ggozad/oterm#188

When the server configuration defines an environment, connecting to the server runs without the default environment, which includes the PATH var. Often uvx, npx, docker will not be found which means that any server will fail to load. This is the case for MacOS & brew for example.

In addition, passing the entire environment (as is the case when the configuration does not define an environment) does involve the security risk of exposing sensitive env variables.

Perhaps setting the default env to include PATH and then updating it with the server config's env, would resolve the issue?

@Kludex
Copy link
Member

Kludex commented Mar 21, 2025

This is going to be fixed in 1.5.0. 🙏

@Kludex Kludex closed this as completed Mar 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

9 participants