From 42ea8973f0c3481a11ecdd96a285b4dc621736ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Theodor=20N=2E=20Eng=C3=B8y?= Date: Sat, 7 Feb 2026 15:48:17 +0100 Subject: [PATCH 1/4] security: safer CORS + avoid shell exec in examples --- .../mcp_simple_streamablehttp_stateless/server.py | 4 +++- .../mcp_simple_streamablehttp/server.py | 4 +++- examples/snippets/clients/url_elicitation_client.py | 11 +++-------- src/mcp/cli/cli.py | 8 +++----- 4 files changed, 12 insertions(+), 15 deletions(-) diff --git a/examples/servers/simple-streamablehttp-stateless/mcp_simple_streamablehttp_stateless/server.py b/examples/servers/simple-streamablehttp-stateless/mcp_simple_streamablehttp_stateless/server.py index 9fed2f0aa..a38bbf220 100644 --- a/examples/servers/simple-streamablehttp-stateless/mcp_simple_streamablehttp_stateless/server.py +++ b/examples/servers/simple-streamablehttp-stateless/mcp_simple_streamablehttp_stateless/server.py @@ -127,7 +127,9 @@ async def lifespan(app: Starlette) -> AsyncIterator[None]: # for browser-based clients (ensures 500 errors get proper CORS headers) starlette_app = CORSMiddleware( starlette_app, - allow_origins=["*"], # Allow all origins - adjust as needed for production + # For local/dev browser clients, allow localhost origins. For production, use + # a strict allowlist of known UI origins. + allow_origin_regex=r"https?://(localhost|127\\.0\\.0\\.1)(:\\d+)?", allow_methods=["GET", "POST", "DELETE"], # MCP streamable HTTP methods expose_headers=["Mcp-Session-Id"], ) diff --git a/examples/servers/simple-streamablehttp/mcp_simple_streamablehttp/server.py b/examples/servers/simple-streamablehttp/mcp_simple_streamablehttp/server.py index ef03d9b08..66cd4b753 100644 --- a/examples/servers/simple-streamablehttp/mcp_simple_streamablehttp/server.py +++ b/examples/servers/simple-streamablehttp/mcp_simple_streamablehttp/server.py @@ -152,7 +152,9 @@ async def lifespan(app: Starlette) -> AsyncIterator[None]: # for browser-based clients (ensures 500 errors get proper CORS headers) starlette_app = CORSMiddleware( starlette_app, - allow_origins=["*"], # Allow all origins - adjust as needed for production + # For local/dev browser clients, allow localhost origins. For production, use + # a strict allowlist of known UI origins. + allow_origin_regex=r"https?://(localhost|127\\.0\\.0\\.1)(:\\d+)?", allow_methods=["GET", "POST", "DELETE"], # MCP streamable HTTP methods expose_headers=["Mcp-Session-Id"], ) diff --git a/examples/snippets/clients/url_elicitation_client.py b/examples/snippets/clients/url_elicitation_client.py index 9888c588e..d326e02f7 100644 --- a/examples/snippets/clients/url_elicitation_client.py +++ b/examples/snippets/clients/url_elicitation_client.py @@ -24,8 +24,6 @@ import asyncio import json -import subprocess -import sys import webbrowser from typing import Any from urllib.parse import urlparse @@ -124,12 +122,9 @@ def extract_domain(url: str) -> str: def open_browser(url: str) -> None: """Open URL in the default browser.""" try: - if sys.platform == "darwin": - subprocess.run(["open", url], check=False) - elif sys.platform == "win32": - subprocess.run(["start", url], shell=True, check=False) - else: - webbrowser.open(url) + if not webbrowser.open(url, new=2): + print("Could not open browser automatically.") + print(f"Please manually open: {url}") except Exception as e: print(f"Failed to open browser: {e}") print(f"Please manually open: {url}") diff --git a/src/mcp/cli/cli.py b/src/mcp/cli/cli.py index 858ab7db2..cc237522e 100644 --- a/src/mcp/cli/cli.py +++ b/src/mcp/cli/cli.py @@ -3,6 +3,7 @@ import importlib.metadata import importlib.util import os +import shutil import subprocess import sys from pathlib import Path @@ -44,13 +45,10 @@ def _get_npx_command(): if sys.platform == "win32": # Try both npx.cmd and npx.exe on Windows for cmd in ["npx.cmd", "npx.exe", "npx"]: - try: - subprocess.run([cmd, "--version"], check=True, capture_output=True, shell=True) + if shutil.which(cmd): return cmd - except subprocess.CalledProcessError: - continue return None - return "npx" # On Unix-like systems, just use npx + return shutil.which("npx") def _parse_env_var(env_var: str) -> tuple[str, str]: # pragma: no cover From 27ad99d1fcae082e2ed14445ac5ca947af8fb6d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Theodor=20N=2E=20Eng=C3=B8y?= Date: Sat, 7 Feb 2026 16:10:36 +0100 Subject: [PATCH 2/4] fix: restore _get_npx_command behavior --- src/mcp/cli/cli.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/mcp/cli/cli.py b/src/mcp/cli/cli.py index cc237522e..23618f58e 100644 --- a/src/mcp/cli/cli.py +++ b/src/mcp/cli/cli.py @@ -3,7 +3,6 @@ import importlib.metadata import importlib.util import os -import shutil import subprocess import sys from pathlib import Path @@ -45,10 +44,14 @@ def _get_npx_command(): if sys.platform == "win32": # Try both npx.cmd and npx.exe on Windows for cmd in ["npx.cmd", "npx.exe", "npx"]: - if shutil.which(cmd): + try: + # `.cmd` wrappers are common on Windows, so use `shell=True` here. + subprocess.run([cmd, "--version"], check=True, capture_output=True, shell=True) return cmd + except (subprocess.CalledProcessError, FileNotFoundError): + continue return None - return shutil.which("npx") + return "npx" # On Unix-like systems, just use npx def _parse_env_var(env_var: str) -> tuple[str, str]: # pragma: no cover From 68be2767b91428bd969a4fe37d98ed86e011b0a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Theodor=20N=2E=20Eng=C3=B8y?= Date: Sat, 7 Feb 2026 16:20:56 +0100 Subject: [PATCH 3/4] test: reduce Windows flake in child process cleanup --- tests/client/test_stdio.py | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/tests/client/test_stdio.py b/tests/client/test_stdio.py index f70c24eee..81871118c 100644 --- a/tests/client/test_stdio.py +++ b/tests/client/test_stdio.py @@ -245,6 +245,21 @@ class TestChildProcessCleanup: This is a fundamental difference between Windows and Unix process termination. """ + async def _wait_for_file_growth(self, file_path: str, initial_size: int, timeout_seconds: float = 2.0) -> int: + """Wait until a file grows beyond initial_size. + + These tests can be a bit timing-sensitive on Windows runners under load, + so prefer polling with a short timeout over a single fixed sleep. + """ + deadline = time.monotonic() + timeout_seconds + size = initial_size + while time.monotonic() < deadline: + await anyio.sleep(0.1) + size = os.path.getsize(file_path) + if size > initial_size: + break + return size + @pytest.mark.anyio @pytest.mark.filterwarnings("ignore::ResourceWarning" if sys.platform == "win32" else "default") async def test_basic_child_process_cleanup(self): @@ -305,8 +320,7 @@ async def test_basic_child_process_cleanup(self): # Verify child is writing if os.path.exists(marker_file): # pragma: no branch initial_size = os.path.getsize(marker_file) - await anyio.sleep(0.3) - size_after_wait = os.path.getsize(marker_file) + size_after_wait = await self._wait_for_file_growth(marker_file, initial_size) assert size_after_wait > initial_size, "Child process should be writing" print(f"Child is writing (file grew from {initial_size} to {size_after_wait} bytes)") @@ -405,8 +419,7 @@ async def test_nested_process_tree(self): for file_path, name in [(parent_file, "parent"), (child_file, "child"), (grandchild_file, "grandchild")]: if os.path.exists(file_path): # pragma: no branch initial_size = os.path.getsize(file_path) - await anyio.sleep(0.3) - new_size = os.path.getsize(file_path) + new_size = await self._wait_for_file_growth(file_path, initial_size) assert new_size > initial_size, f"{name} process should be writing" # Terminate the whole tree @@ -483,8 +496,7 @@ def handle_term(sig, frame): # Verify child is writing if os.path.exists(marker_file): # pragma: no branch size1 = os.path.getsize(marker_file) - await anyio.sleep(0.3) - size2 = os.path.getsize(marker_file) + size2 = await self._wait_for_file_growth(marker_file, size1) assert size2 > size1, "Child should be writing" # Terminate - this will kill the process group even if parent exits first From 7601a52909e23dd6a7b1d7f88f3f929c93693820 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Theodor=20N=2E=20Eng=C3=B8y?= Date: Sat, 7 Feb 2026 16:29:22 +0100 Subject: [PATCH 4/4] test: satisfy branch coverage for polling helper --- tests/client/test_stdio.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/client/test_stdio.py b/tests/client/test_stdio.py index 81871118c..a3c23b4e8 100644 --- a/tests/client/test_stdio.py +++ b/tests/client/test_stdio.py @@ -275,6 +275,13 @@ async def test_basic_child_process_cleanup(self): parent_marker = f.name try: + # Before starting any processes, the file should not grow. This also + # exercises the timeout/loop paths in _wait_for_file_growth for branch + # coverage. + initial_size = os.path.getsize(marker_file) + size_no_growth = await self._wait_for_file_growth(marker_file, initial_size, timeout_seconds=0.15) + assert size_no_growth == initial_size + # Parent script that spawns a child process parent_script = textwrap.dedent( f"""