python-sdk icon indicating copy to clipboard operation
python-sdk copied to clipboard

MCP Client Environment Variables in StdioServerParameters

Open AshuJoshi opened this issue 1 year ago • 1 comments

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.

AshuJoshi avatar Dec 11 '24 19:12 AshuJoshi

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: https://github.com/modelcontextprotocol/python-sdk/blob/aaf32b530738ff79ba607c2884374243350f521c/src/mcp/client/stdio.py#L13-L51

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...

christopherhwood avatar Dec 12 '24 00:12 christopherhwood

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

kzmszk avatar Jan 10 '25 08:01 kzmszk

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")

BryceShakeAir avatar Jan 12 '25 06:01 BryceShakeAir

Oh I just saw this and I found similar errors and I proposed some code in https://github.com/modelcontextprotocol/python-sdk/issues/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,
)

zhjch05 avatar Feb 03 '25 13:02 zhjch05

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

sbshah97 avatar Mar 01 '25 14:03 sbshah97

I stumbled upon this because of https://github.com/ggozad/oterm/issues/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?

ggozad avatar Mar 15 '25 23:03 ggozad

This is going to be fixed in 1.5.0. 🙏

Kludex avatar Mar 21 '25 11:03 Kludex

This was fixed but not taking @ggozad's comment into account. Specifically, with this fix, stdio-based MCPs have access to all the env variables, including credentials and tokens of any locally running server.

Just wanted to leave a note for awareness, since it's probably not worth digging this out as we're apparently moving away from stdio anyway.

kepler avatar Jun 12 '25 08:06 kepler

This was fixed but not taking @ggozad's comment into account. Specifically, with this fix, stdio-based MCPs have access to all the env variables, including credentials and tokens of any locally running server.

Just wanted to leave a note for awareness, since it's probably not worth digging this out as we're apparently moving away from stdio anyway.

This is not true. This is not what is implemented.

Kludex avatar Jun 12 '25 08:06 Kludex

I stand corrected. Apologies for the misinformation and thank you for pointing it out.

The env vars are indeed filtered out: https://github.com/modelcontextprotocol/python-sdk/blob/0bcecffc4c52c52e0502e53b59715ebaba88f998/src/mcp/client/stdio/init.py#L23-L39

It's a framework I'm using that uses MCP and injects the whole os.environ with or without the user passing in an env.

kepler avatar Jun 12 '25 11:06 kepler

All good. I just really corrected for awareness. 👀 👍

Kludex avatar Jun 12 '25 11:06 Kludex