Skip to content

Commit 90b1aa0

Browse files
committed
fix: prevent command injection in example URL opening
Replace platform-specific subprocess calls with webbrowser.open() and add URL scheme validation to the elicitation example client. The previous Windows code path used shell=True with subprocess, which allowed command injection via crafted URLs containing shell metacharacters (e.g., & as a command separator in cmd.exe). Changes: - Remove subprocess/sys imports, use webbrowser.open() for all platforms - Add URL scheme allowlist (http/https only) to prevent abuse via dangerous protocol handlers (file://, smb://, ms-msdt://, etc.) - Align with the safe pattern already used in the OAuth example client
1 parent fc57c2c commit 90b1aa0

File tree

1 file changed

+9
-9
lines changed

1 file changed

+9
-9
lines changed

examples/snippets/clients/url_elicitation_client.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@
2424

2525
import asyncio
2626
import json
27-
import subprocess
28-
import sys
2927
import webbrowser
3028
from typing import Any
3129
from urllib.parse import urlparse
@@ -121,15 +119,17 @@ def extract_domain(url: str) -> str:
121119
return "unknown"
122120

123121

122+
ALLOWED_SCHEMES = {"http", "https"}
123+
124+
124125
def open_browser(url: str) -> None:
125-
"""Open URL in the default browser."""
126+
"""Open URL in the default browser after validating the scheme."""
127+
parsed = urlparse(url)
128+
if parsed.scheme.lower() not in ALLOWED_SCHEMES:
129+
print(f"Refusing to open URL with unsupported scheme '{parsed.scheme}': {url}")
130+
return
126131
try:
127-
if sys.platform == "darwin":
128-
subprocess.run(["open", url], check=False)
129-
elif sys.platform == "win32":
130-
subprocess.run(["start", url], shell=True, check=False)
131-
else:
132-
webbrowser.open(url)
132+
webbrowser.open(url)
133133
except Exception as e:
134134
print(f"Failed to open browser: {e}")
135135
print(f"Please manually open: {url}")

0 commit comments

Comments
 (0)