mirror of
https://github.com/langflow-ai/langflow.git
synced 2026-07-25 22:19:40 +08:00
add windows const paths
This commit is contained in:
@ -105,6 +105,7 @@ async def auto_configure_default_mcp_servers(session: AsyncSession) -> None:
|
||||
"mcpServers", {}
|
||||
)
|
||||
persisted = existing_servers.get(server_name)
|
||||
is_reconcile = False
|
||||
if persisted is not None:
|
||||
persisted_meta = persisted.get("metadata") or {}
|
||||
if not persisted_meta.get("auto_configured"):
|
||||
@ -124,18 +125,10 @@ async def auto_configure_default_mcp_servers(session: AsyncSession) -> None:
|
||||
reason="already_in_sync",
|
||||
)
|
||||
continue
|
||||
# Spec drifted (e.g. user upgraded across a commit that changed the
|
||||
# canonical payload). Reconcile by overwriting with the new spec —
|
||||
# `auto_configured: True` was our marker that the entry is ours.
|
||||
# Platform context lets ops correlate reconciliations against the
|
||||
# reported OS when the prior version's payload is too short for
|
||||
# that OS (notably first-run npx timeouts on Windows).
|
||||
await logger.ainfo(
|
||||
"default_mcp_server_reconciled",
|
||||
user_id=str(user.id),
|
||||
server_name=server_name,
|
||||
platform=platform.system(),
|
||||
)
|
||||
# Spec drifted (e.g. user upgraded across a commit that changed
|
||||
# the canonical payload). `auto_configured: True` was our marker
|
||||
# that the entry is ours, so overwriting is safe.
|
||||
is_reconcile = True
|
||||
await update_server(
|
||||
server_name=server_name,
|
||||
server_config=payload,
|
||||
@ -144,11 +137,23 @@ async def auto_configure_default_mcp_servers(session: AsyncSession) -> None:
|
||||
storage_service=storage_service,
|
||||
settings_service=settings_service,
|
||||
)
|
||||
await logger.ainfo(
|
||||
"default_mcp_server_added",
|
||||
user_id=str(user.id),
|
||||
server_name=server_name,
|
||||
)
|
||||
# Distinct event names so Sentry/Datadog filtering can separate
|
||||
# fresh installs from upgrade-driven reconciliations. Platform is
|
||||
# included on reconcile because the bug that motivated this path
|
||||
# (first-run `npx -y` timeouts on Windows) is OS-correlated.
|
||||
if is_reconcile:
|
||||
await logger.ainfo(
|
||||
"default_mcp_server_reconciled",
|
||||
user_id=str(user.id),
|
||||
server_name=server_name,
|
||||
platform=platform.system(),
|
||||
)
|
||||
else:
|
||||
await logger.ainfo(
|
||||
"default_mcp_server_added",
|
||||
user_id=str(user.id),
|
||||
server_name=server_name,
|
||||
)
|
||||
except (
|
||||
HTTPException,
|
||||
sqlalchemy_exc.SQLAlchemyError,
|
||||
|
||||
@ -0,0 +1,198 @@
|
||||
"""Tests for OS-essential env var passthrough in MCP stdio connections.
|
||||
|
||||
Why this exists: child MCP servers (notably `@wonderwhy-er/desktop-commander`)
|
||||
call Node's `os.homedir()` at module load. On Windows, `os.homedir()` reads
|
||||
`USERPROFILE` (or `HOMEDRIVE`+`HOMEPATH`); on Unix it reads `HOME` first and
|
||||
only then falls back to a `getpwuid()` lookup.
|
||||
|
||||
Before this fix, `MCPStdioClient._connect_to_server` passed only `{DEBUG, PATH}`
|
||||
into the subprocess env. On Windows that left `os.homedir()` returning `null`,
|
||||
which then crashed any server that built paths from `path.join(USER_HOME, ...)`.
|
||||
The MCP SDK then surfaced the crash as an opaque "Connection closed" /
|
||||
TaskGroup sub-exception — same error users saw for the shell-execution server.
|
||||
|
||||
This file pins the curated passthrough so the next person who tightens the env
|
||||
list doesn't accidentally re-strip `USERPROFILE` / `HOME` and resurrect the
|
||||
Windows bug.
|
||||
"""
|
||||
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import pytest
|
||||
from lfx.base.mcp.util import MCPStdioClient
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def stdio_client():
|
||||
return MCPStdioClient()
|
||||
|
||||
|
||||
async def _connect_capturing_env(client: MCPStdioClient, *, command: str = "echo hi") -> dict[str, str]:
|
||||
"""Capture the env dict the StdioServerParameters was built with.
|
||||
|
||||
Drives `_connect_to_server` far enough that StdioServerParameters is built,
|
||||
then returns the env dict — without launching a subprocess.
|
||||
"""
|
||||
with patch.object(
|
||||
client,
|
||||
"_get_or_create_session",
|
||||
new=AsyncMock(return_value=SimpleNamespace(list_tools=AsyncMock(return_value=SimpleNamespace(tools=[])))),
|
||||
):
|
||||
await client._connect_to_server(command)
|
||||
return dict(client._connection_params.env)
|
||||
|
||||
|
||||
class TestWindowsEnvPassthrough:
|
||||
"""When the host is Windows, `USERPROFILE` etc must reach the child."""
|
||||
|
||||
async def test_should_pass_userprofile_when_present_on_windows(self, stdio_client):
|
||||
with (
|
||||
patch("lfx.base.mcp.util.platform.system", return_value="Windows"),
|
||||
patch.dict(
|
||||
"os.environ",
|
||||
{"PATH": "C:/Windows", "USERPROFILE": r"C:\Users\test"},
|
||||
clear=True,
|
||||
),
|
||||
):
|
||||
env = await _connect_capturing_env(stdio_client)
|
||||
assert env.get("USERPROFILE") == r"C:\Users\test"
|
||||
|
||||
async def test_should_pass_homedrive_and_homepath_fallbacks_on_windows(self, stdio_client):
|
||||
"""`os.homedir()` falls back to HOMEDRIVE+HOMEPATH when USERPROFILE is absent."""
|
||||
with (
|
||||
patch("lfx.base.mcp.util.platform.system", return_value="Windows"),
|
||||
patch.dict(
|
||||
"os.environ",
|
||||
{"PATH": "C:/Windows", "HOMEDRIVE": "C:", "HOMEPATH": r"\Users\test"},
|
||||
clear=True,
|
||||
),
|
||||
):
|
||||
env = await _connect_capturing_env(stdio_client)
|
||||
assert env.get("HOMEDRIVE") == "C:"
|
||||
assert env.get("HOMEPATH") == r"\Users\test"
|
||||
|
||||
async def test_should_pass_appdata_and_temp_on_windows(self, stdio_client):
|
||||
with (
|
||||
patch("lfx.base.mcp.util.platform.system", return_value="Windows"),
|
||||
patch.dict(
|
||||
"os.environ",
|
||||
{
|
||||
"PATH": "C:/Windows",
|
||||
"APPDATA": r"C:\Users\test\AppData\Roaming",
|
||||
"LOCALAPPDATA": r"C:\Users\test\AppData\Local",
|
||||
"TEMP": r"C:\Users\test\AppData\Local\Temp",
|
||||
"TMP": r"C:\Users\test\AppData\Local\Temp",
|
||||
},
|
||||
clear=True,
|
||||
),
|
||||
):
|
||||
env = await _connect_capturing_env(stdio_client)
|
||||
assert env.get("APPDATA", "").endswith("Roaming")
|
||||
assert env.get("LOCALAPPDATA", "").endswith("Local")
|
||||
assert env.get("TEMP", "").endswith("Temp")
|
||||
assert env.get("TMP", "").endswith("Temp")
|
||||
|
||||
async def test_should_pass_pathext_so_cmd_resolves_npx_cmd_on_windows(self, stdio_client):
|
||||
"""Without PATHEXT, `cmd /c npx ...` can't find `npx.cmd` in PATH."""
|
||||
with (
|
||||
patch("lfx.base.mcp.util.platform.system", return_value="Windows"),
|
||||
patch.dict(
|
||||
"os.environ",
|
||||
{"PATH": "C:/Windows", "PATHEXT": ".COM;.EXE;.BAT;.CMD"},
|
||||
clear=True,
|
||||
),
|
||||
):
|
||||
env = await _connect_capturing_env(stdio_client)
|
||||
assert ".CMD" in env.get("PATHEXT", "")
|
||||
|
||||
async def test_should_omit_windows_vars_when_absent_from_host_environ(self, stdio_client):
|
||||
"""Only forward what's actually present.
|
||||
|
||||
If the host doesn't have a Windows var (e.g. running tests on Unix),
|
||||
we don't fabricate one.
|
||||
"""
|
||||
with (
|
||||
patch("lfx.base.mcp.util.platform.system", return_value="Windows"),
|
||||
patch.dict("os.environ", {"PATH": "C:/Windows"}, clear=True),
|
||||
):
|
||||
env = await _connect_capturing_env(stdio_client)
|
||||
assert "USERPROFILE" not in env
|
||||
assert "APPDATA" not in env
|
||||
|
||||
|
||||
class TestUnixEnvPassthrough:
|
||||
"""Forward Unix-essential env vars to the subprocess.
|
||||
|
||||
On Unix, `HOME` falls back to `getpwuid()` but many tools still read it
|
||||
from env first. Forwarding it avoids per-tool quirks.
|
||||
"""
|
||||
|
||||
async def test_should_pass_home_when_present_on_unix(self, stdio_client):
|
||||
with (
|
||||
patch("lfx.base.mcp.util.platform.system", return_value="Linux"),
|
||||
patch.dict("os.environ", {"PATH": "/usr/bin", "HOME": "/home/test"}, clear=True),
|
||||
):
|
||||
env = await _connect_capturing_env(stdio_client)
|
||||
assert env.get("HOME") == "/home/test"
|
||||
|
||||
async def test_should_pass_lang_for_locale_aware_tools_on_unix(self, stdio_client):
|
||||
with (
|
||||
patch("lfx.base.mcp.util.platform.system", return_value="Darwin"),
|
||||
patch.dict(
|
||||
"os.environ",
|
||||
{"PATH": "/usr/bin", "HOME": "/Users/test", "LANG": "en_US.UTF-8"},
|
||||
clear=True,
|
||||
),
|
||||
):
|
||||
env = await _connect_capturing_env(stdio_client)
|
||||
assert env.get("LANG") == "en_US.UTF-8"
|
||||
|
||||
async def test_should_not_pass_windows_vars_when_running_on_unix(self, stdio_client):
|
||||
"""Windows-only vars must not bleed through on Unix hosts."""
|
||||
with (
|
||||
patch("lfx.base.mcp.util.platform.system", return_value="Linux"),
|
||||
patch.dict(
|
||||
"os.environ",
|
||||
{"PATH": "/usr/bin", "HOME": "/h", "USERPROFILE": "should-not-leak"},
|
||||
clear=True,
|
||||
),
|
||||
):
|
||||
env = await _connect_capturing_env(stdio_client)
|
||||
assert "USERPROFILE" not in env
|
||||
|
||||
|
||||
class TestUserSuppliedEnvWins:
|
||||
"""User-supplied env (from server config) must override host passthrough."""
|
||||
|
||||
async def test_should_let_user_env_override_passthrough_value(self, stdio_client):
|
||||
with (
|
||||
patch("lfx.base.mcp.util.platform.system", return_value="Linux"),
|
||||
patch.dict(
|
||||
"os.environ",
|
||||
{"PATH": "/usr/bin", "HOME": "/host/home"},
|
||||
clear=True,
|
||||
),
|
||||
patch.object(
|
||||
stdio_client,
|
||||
"_get_or_create_session",
|
||||
new=AsyncMock(
|
||||
return_value=SimpleNamespace(list_tools=AsyncMock(return_value=SimpleNamespace(tools=[])))
|
||||
),
|
||||
),
|
||||
):
|
||||
await stdio_client._connect_to_server("echo hi", env={"HOME": "/user/override"})
|
||||
assert dict(stdio_client._connection_params.env).get("HOME") == "/user/override"
|
||||
|
||||
|
||||
class TestLegacyEnvKeysPreserved:
|
||||
"""Don't drop keys the previous behavior guaranteed (avoids silent regressions)."""
|
||||
|
||||
async def test_should_still_pass_path_and_debug(self, stdio_client):
|
||||
with (
|
||||
patch("lfx.base.mcp.util.platform.system", return_value="Linux"),
|
||||
patch.dict("os.environ", {"PATH": "/usr/bin"}, clear=True),
|
||||
):
|
||||
env = await _connect_capturing_env(stdio_client)
|
||||
assert env.get("PATH") == "/usr/bin"
|
||||
assert env.get("DEBUG") == "true"
|
||||
@ -1547,6 +1547,73 @@ class MCPSessionManager:
|
||||
self._context_to_session.pop(context_id, None)
|
||||
|
||||
|
||||
# OS-essential env vars to forward into MCP stdio subprocesses.
|
||||
#
|
||||
# Why this list exists: Node-based MCP servers (e.g. `@wonderwhy-er/desktop-commander`)
|
||||
# call `os.homedir()` at module load. On Windows that reads `USERPROFILE` (or the
|
||||
# `HOMEDRIVE`+`HOMEPATH` fallback); when the var is missing the call returns
|
||||
# `null` and any `path.join(homedir, ...)` blows up — surfacing to MCP clients
|
||||
# as an opaque "Connection closed" / TaskGroup sub-exception. The same is true
|
||||
# of locale, temp-dir, and shell-related vars for other server families.
|
||||
#
|
||||
# Why not pass `os.environ` wholesale: the parent Langflow process may carry
|
||||
# secrets (AWS_*, GITHUB_TOKEN, OPENAI_API_KEY, ...) that have no business
|
||||
# being inherited by an arbitrary MCP server subprocess. We curate.
|
||||
#
|
||||
# User-supplied env (from server config) still wins via merge order.
|
||||
_PASSTHROUGH_ENV_VARS_WINDOWS: tuple[str, ...] = (
|
||||
# `os.homedir()` lookup chain on Windows.
|
||||
"USERPROFILE",
|
||||
"HOMEDRIVE",
|
||||
"HOMEPATH",
|
||||
# Per-user app data directories — many MCP servers store config here.
|
||||
"APPDATA",
|
||||
"LOCALAPPDATA",
|
||||
# Temp dirs — required by tools that spawn helper subprocesses.
|
||||
"TEMP",
|
||||
"TMP",
|
||||
# Required for `cmd /c` and child Win32 APIs.
|
||||
"SYSTEMROOT",
|
||||
"SYSTEMDRIVE",
|
||||
"WINDIR",
|
||||
"COMSPEC",
|
||||
# Without PATHEXT, `cmd /c npx ...` cannot resolve `npx.cmd` from PATH.
|
||||
"PATHEXT",
|
||||
)
|
||||
_PASSTHROUGH_ENV_VARS_UNIX: tuple[str, ...] = (
|
||||
# `os.homedir()` reads HOME first; getpwuid() is only the fallback.
|
||||
"HOME",
|
||||
"USER",
|
||||
"LOGNAME",
|
||||
# Shell-aware tools.
|
||||
"SHELL",
|
||||
"TERM",
|
||||
"TMPDIR",
|
||||
# Locale — affects text encoding, sort order, error messages.
|
||||
"LANG",
|
||||
"LC_ALL",
|
||||
"LC_CTYPE",
|
||||
)
|
||||
|
||||
|
||||
def _build_subprocess_env(user_env: dict[str, str] | None) -> dict[str, str]:
|
||||
"""Build the env dict passed to MCP stdio subprocesses.
|
||||
|
||||
Always-set keys: ``DEBUG=true`` (legacy) and ``PATH`` (so tools resolve).
|
||||
Forwarded if present in the parent env: a curated list of OS-essential
|
||||
vars (``USERPROFILE`` etc on Windows, ``HOME`` etc on Unix). Anything in
|
||||
``user_env`` overrides everything else.
|
||||
"""
|
||||
passthrough = _PASSTHROUGH_ENV_VARS_WINDOWS if platform.system() == "Windows" else _PASSTHROUGH_ENV_VARS_UNIX
|
||||
env: dict[str, str] = {"DEBUG": "true", "PATH": os.environ.get("PATH", "")}
|
||||
for var in passthrough:
|
||||
if var in os.environ:
|
||||
env[var] = os.environ[var]
|
||||
if user_env:
|
||||
env.update(user_env)
|
||||
return env
|
||||
|
||||
|
||||
class MCPStdioClient:
|
||||
def __init__(self, component_cache=None):
|
||||
self.session: ClientSession | None = None
|
||||
@ -1576,7 +1643,7 @@ class MCPStdioClient:
|
||||
from mcp import StdioServerParameters
|
||||
|
||||
command = shlex.split(command_str)
|
||||
env_data: dict[str, str] = {"DEBUG": "true", "PATH": os.environ["PATH"], **(env or {})}
|
||||
env_data: dict[str, str] = _build_subprocess_env(env)
|
||||
|
||||
if platform.system() == "Windows":
|
||||
server_params = StdioServerParameters(
|
||||
|
||||
Reference in New Issue
Block a user