Skip to content

Commit dc1cf19

Browse files
committed
fix: remove shell=True from subprocess calls in mcp dev (command injection risk)
Remove shell=True from _get_npx_command() and the MCP Inspector subprocess.run() call. On Windows, _get_npx_command() already resolves to the correct .cmd/.exe extension, so shell=True is unnecessary and exposes a command injection risk via shell metacharacters in file paths. Also catch FileNotFoundError in _get_npx_command() for robustness when the command is not found without shell expansion.
1 parent 62575ed commit dc1cf19

1 file changed

Lines changed: 3 additions & 5 deletions

File tree

src/mcp/cli/cli.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,9 @@ def _get_npx_command():
4545
# Try both npx.cmd and npx.exe on Windows
4646
for cmd in ["npx.cmd", "npx.exe", "npx"]:
4747
try:
48-
subprocess.run([cmd, "--version"], check=True, capture_output=True, shell=True)
48+
subprocess.run([cmd, "--version"], check=True, capture_output=True)
4949
return cmd
50-
except subprocess.CalledProcessError:
50+
except (subprocess.CalledProcessError, FileNotFoundError):
5151
continue
5252
return None
5353
return "npx" # On Unix-like systems, just use npx
@@ -271,12 +271,10 @@ def dev(
271271
)
272272
sys.exit(1)
273273

274-
# Run the MCP Inspector command with shell=True on Windows
275-
shell = sys.platform == "win32"
274+
# Run the MCP Inspector command
276275
process = subprocess.run(
277276
[npx_cmd, "@modelcontextprotocol/inspector"] + uv_cmd,
278277
check=True,
279-
shell=shell,
280278
env=dict(os.environ.items()), # Convert to list of tuples for env update
281279
)
282280
sys.exit(process.returncode)

0 commit comments

Comments
 (0)