diff --git a/src/fastmcp/cli/cli.py b/src/fastmcp/cli/cli.py index d5f373db0..42f582b1c 100644 --- a/src/fastmcp/cli/cli.py +++ b/src/fastmcp/cli/cli.py @@ -46,9 +46,7 @@ def _get_npx_command(): # 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 - ) + subprocess.run([cmd, "--version"], check=True, capture_output=True) return cmd except subprocess.CalledProcessError: continue @@ -277,12 +275,10 @@ async def dev( # Set marker to prevent infinite loops when subprocess calls FastMCP env = dict(os.environ.items()) | env_vars | {"FASTMCP_UV_SPAWNED": "1"} - # Run the MCP Inspector command with shell=True on Windows - shell = sys.platform == "win32" + # Run the MCP Inspector command process = subprocess.run( [npx_cmd, inspector_cmd] + uv_cmd, check=True, - shell=shell, env=env, ) sys.exit(process.returncode) diff --git a/src/fastmcp/cli/install/cursor.py b/src/fastmcp/cli/install/cursor.py index dd885e5ea..ee0edb2c6 100644 --- a/src/fastmcp/cli/install/cursor.py +++ b/src/fastmcp/cli/install/cursor.py @@ -56,7 +56,7 @@ def open_deeplink(deeplink: str) -> bool: subprocess.run(["open", deeplink], check=True, capture_output=True) elif sys.platform == "win32": # Windows subprocess.run( - ["start", deeplink], shell=True, check=True, capture_output=True + ["cmd", "/c", "start", deeplink], check=True, capture_output=True ) else: # Linux and others subprocess.run(["xdg-open", deeplink], check=True, capture_output=True) diff --git a/src/fastmcp/client/oauth_callback.py b/src/fastmcp/client/oauth_callback.py index cf1166a80..ced483ea2 100644 --- a/src/fastmcp/client/oauth_callback.py +++ b/src/fastmcp/client/oauth_callback.py @@ -46,9 +46,7 @@ def create_callback_html( # Add detail info box for both success and error cases detail_info = "" if is_success and server_url: - detail_info = create_info_box( - f"Connected to: {server_url}", centered=True - ) + detail_info = create_info_box(f"Connected to: {server_url}", centered=True) elif not is_success: detail_info = create_info_box(message, is_error=True, centered=True) diff --git a/src/fastmcp/server/auth/providers/jwt.py b/src/fastmcp/server/auth/providers/jwt.py index c33d122ef..552654ff7 100644 --- a/src/fastmcp/server/auth/providers/jwt.py +++ b/src/fastmcp/server/auth/providers/jwt.py @@ -382,7 +382,12 @@ class JWTVerifier(TokenVerifier): claims = self.jwt.decode(token, verification_key) # Extract client ID early for logging - client_id = claims.get("client_id") or claims.get("sub") or "unknown" + client_id = ( + claims.get("client_id") + or claims.get("azp") + or claims.get("sub") + or "unknown" + ) # Validate expiration exp = claims.get("exp") diff --git a/src/fastmcp/utilities/ui.py b/src/fastmcp/utilities/ui.py index 0d5c3bafd..e5a8429a2 100644 --- a/src/fastmcp/utilities/ui.py +++ b/src/fastmcp/utilities/ui.py @@ -7,6 +7,8 @@ consent pages, and other user-facing interfaces. from __future__ import annotations +import html + from starlette.responses import HTMLResponse # FastMCP branding @@ -339,6 +341,7 @@ def create_page( Returns: Complete HTML page as string """ + title = html.escape(title) return f""" @@ -375,6 +378,7 @@ def create_status_message(message: str, is_success: bool = True) -> str: Returns: HTML for status message """ + message = html.escape(message) icon = "✓" if is_success else "✕" icon_class = "success" if is_success else "error" @@ -400,6 +404,7 @@ def create_info_box( Returns: HTML for info box """ + content = html.escape(content) classes = ["info-box"] if is_error: classes.append("error") @@ -422,8 +427,8 @@ def create_detail_box(rows: list[tuple[str, str]]) -> str: rows_html = "\n".join( f"""
-
{label}:
-
{value}
+
{html.escape(label)}:
+
{html.escape(value)}
""" for label, value in rows diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index 01e52ef0e..10e86b8a7 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -416,7 +416,6 @@ class TestWindowsSpecific: ["npx.cmd", "--version"], check=True, capture_output=True, - shell=True, ) @patch("subprocess.run") diff --git a/tests/cli/test_cursor.py b/tests/cli/test_cursor.py index df40a7eab..a1af8241d 100644 --- a/tests/cli/test_cursor.py +++ b/tests/cli/test_cursor.py @@ -145,7 +145,7 @@ class TestOpenDeeplink: assert result is True mock_run.assert_called_once_with( - ["start", "cursor://test"], shell=True, check=True, capture_output=True + ["cmd", "/c", "start", "cursor://test"], check=True, capture_output=True ) @patch("subprocess.run") diff --git a/tests/client/test_oauth_callback_xss.py b/tests/client/test_oauth_callback_xss.py new file mode 100644 index 000000000..626fc7798 --- /dev/null +++ b/tests/client/test_oauth_callback_xss.py @@ -0,0 +1,159 @@ +"""Comprehensive XSS protection tests for OAuth callback HTML rendering.""" + +import pytest + +from fastmcp.client.oauth_callback import create_callback_html +from fastmcp.utilities.ui import ( + create_detail_box, + create_info_box, + create_page, + create_status_message, +) + + +def test_ui_create_page_escapes_title(): + """Test that page title is properly escaped.""" + xss_title = "" + html = create_page("content", title=xss_title) + assert "<script>alert(1)</script>" in html + assert "" not in html + + +def test_ui_create_status_message_escapes(): + """Test that status messages are properly escaped.""" + xss_message = "" + html = create_status_message(xss_message) + assert "<img src=x onerror=alert(1)>" in html + assert "" not in html + + +def test_ui_create_info_box_escapes(): + """Test that info box content is properly escaped.""" + xss_content = "" + html = create_info_box(xss_content) + assert "<iframe" in html + assert "