diff --git a/src/fastmcp/cli/install/claude_code.py b/src/fastmcp/cli/install/claude_code.py index eb6d53745..83e5ee722 100644 --- a/src/fastmcp/cli/install/claude_code.py +++ b/src/fastmcp/cli/install/claude_code.py @@ -1,5 +1,6 @@ """Claude Code integration for FastMCP install using Cyclopts.""" +import shlex import shutil import subprocess import sys @@ -124,8 +125,8 @@ def install_claude_code( # Build the full command full_command = env_config.build_command(["fastmcp", "run", server_spec]) - # Build claude mcp add command - cmd_parts = [claude_cmd, "mcp", "add", name] + # Build claude mcp add command (escape name to prevent shell injection) + cmd_parts = [claude_cmd, "mcp", "add", shlex.quote(name)] # Add environment variables if specified if env_vars: diff --git a/src/fastmcp/cli/install/gemini_cli.py b/src/fastmcp/cli/install/gemini_cli.py index 8acaa413e..cf2260c50 100644 --- a/src/fastmcp/cli/install/gemini_cli.py +++ b/src/fastmcp/cli/install/gemini_cli.py @@ -1,5 +1,6 @@ """Gemini CLI integration for FastMCP install using Cyclopts.""" +import shlex import shutil import subprocess import sys @@ -129,8 +130,8 @@ def install_gemini_cli( for key, value in env_vars.items(): cmd_parts.extend(["-e", f"{key}={value}"]) - # Add server name and command - cmd_parts.extend([name, full_command[0], "--"]) + # Add server name and command (escape name to prevent shell injection) + cmd_parts.extend([shlex.quote(name), full_command[0], "--"]) cmd_parts.extend(full_command[1:]) try: diff --git a/tests/cli/test_install.py b/tests/cli/test_install.py index 6cffefa78..c946f0ecc 100644 --- a/tests/cli/test_install.py +++ b/tests/cli/test_install.py @@ -1,4 +1,6 @@ +import shlex from pathlib import Path +from unittest.mock import MagicMock, patch from fastmcp.cli.install import install_app @@ -311,3 +313,91 @@ def test_project_option(self): command, bound, _ = install_app.parse_args(cmd_args) assert command is not None assert str(bound.arguments["project"]) == str(Path("/path/to/project")) + + +class TestShellInjectionProtection: + """Test that install commands properly escape server names to prevent shell injection.""" + + @patch("fastmcp.cli.install.gemini_cli.find_gemini_command") + @patch("fastmcp.cli.install.gemini_cli.subprocess.run") + def test_gemini_cli_escapes_shell_metacharacters( + self, mock_subprocess, mock_find_gemini + ): + """Test that gemini-cli install properly escapes server names with shell metacharacters.""" + from fastmcp.cli.install.gemini_cli import install_gemini_cli + + # Mock gemini command as available + mock_find_gemini.return_value = "/usr/bin/gemini" + mock_subprocess.return_value = MagicMock(returncode=0) + + # Test with various dangerous characters + dangerous_names = ["test&calc", "test|whoami", "test;ls", "test$USER"] + + for dangerous_name in dangerous_names: + mock_subprocess.reset_mock() + + # Call install function + install_gemini_cli( + file=Path("/tmp/server.py"), + server_object=None, + name=dangerous_name, + ) + + # Verify subprocess was called + assert mock_subprocess.called + cmd_parts = mock_subprocess.call_args[0][0] + + # Verify the name was shell-quoted + # shlex.quote() wraps strings with special chars in single quotes + expected_quoted = shlex.quote(dangerous_name) + assert expected_quoted in cmd_parts, ( + f"Expected {expected_quoted} in command but got {cmd_parts}" + ) + + # Verify the original unquoted name is NOT in the command + # (unless it happens to be the same as the quoted version) + if expected_quoted != dangerous_name: + assert dangerous_name not in cmd_parts, ( + f"Unquoted name {dangerous_name} should not appear in command" + ) + + @patch("fastmcp.cli.install.claude_code.find_claude_command") + @patch("fastmcp.cli.install.claude_code.subprocess.run") + def test_claude_code_escapes_shell_metacharacters( + self, mock_subprocess, mock_find_claude + ): + """Test that claude-code install properly escapes server names with shell metacharacters.""" + from fastmcp.cli.install.claude_code import install_claude_code + + # Mock claude command as available + mock_find_claude.return_value = "/usr/bin/claude" + mock_subprocess.return_value = MagicMock(returncode=0) + + # Test with various dangerous characters + dangerous_names = ["test&calc", "test|whoami", "test;ls", "test$USER"] + + for dangerous_name in dangerous_names: + mock_subprocess.reset_mock() + + # Call install function + install_claude_code( + file=Path("/tmp/server.py"), + server_object=None, + name=dangerous_name, + ) + + # Verify subprocess was called + assert mock_subprocess.called + cmd_parts = mock_subprocess.call_args[0][0] + + # Verify the name was shell-quoted + expected_quoted = shlex.quote(dangerous_name) + assert expected_quoted in cmd_parts, ( + f"Expected {expected_quoted} in command but got {cmd_parts}" + ) + + # Verify the original unquoted name is NOT in the command + if expected_quoted != dangerous_name: + assert dangerous_name not in cmd_parts, ( + f"Unquoted name {dangerous_name} should not appear in command" + )