diff --git a/server/services/dev_server_manager.py b/server/services/dev_server_manager.py index 5acfbc8..5f8dfe8 100644 --- a/server/services/dev_server_manager.py +++ b/server/services/dev_server_manager.py @@ -17,6 +17,7 @@ import re import subprocess import sys import threading +import shlex from datetime import datetime from pathlib import Path from typing import Awaitable, Callable, Literal, Set @@ -24,6 +25,7 @@ from typing import Awaitable, Callable, Literal, Set import psutil from registry import list_registered_projects +from security import extract_commands, get_effective_commands, is_command_allowed from server.utils.process_utils import kill_process_tree logger = logging.getLogger(__name__) @@ -114,7 +116,8 @@ class DevServerProcessManager: self._callbacks_lock = threading.Lock() # Lock file to prevent multiple instances (stored in project directory) - self.lock_file = self.project_dir / ".devserver.lock" + from autocoder_paths import get_devserver_lock_path + self.lock_file = get_devserver_lock_path(self.project_dir) @property def status(self) -> Literal["stopped", "running", "crashed"]: @@ -289,39 +292,47 @@ class DevServerProcessManager: Start the dev server as a subprocess. Args: - command: The shell command to run (e.g., "npm run dev") + command: The command to run (e.g., "npm run dev") Returns: Tuple of (success, message) """ - if self.status == "running": + # Already running? + if self.process and self.status == "running": return False, "Dev server is already running" + # Lock check (prevents double-start) if not self._check_lock(): - return False, "Another dev server instance is already running for this project" + return False, "Dev server already running (lock file present)" - # Validate that project directory exists - if not self.project_dir.exists(): - return False, f"Project directory does not exist: {self.project_dir}" + command = (command or "").strip() + if not command: + return False, "Empty dev server command" - self._command = command - self._detected_url = None # Reset URL detection + # SECURITY: block shell operators/metacharacters (defense-in-depth) + dangerous_ops = ["&&", "||", ";", "|", "`", "$("] + if any(op in command for op in dangerous_ops): + return False, "Shell operators are not allowed in dev server command" + + # Parse into argv and execute without shell + argv = shlex.split(command, posix=(sys.platform != "win32")) + if not argv: + return False, "Empty dev server command" + + base = Path(argv[0]).name.lower() + + # Defense-in-depth: reject direct shells/interpreters commonly used for injection + if base in {"sh", "bash", "zsh", "cmd", "powershell", "pwsh"}: + return False, f"Shell runner '{base}' is not allowed for dev server commands" + + # Windows: use .cmd shims for Node package managers + if sys.platform == "win32" and base in {"npm", "pnpm", "yarn", "npx"} and not argv[0].lower().endswith(".cmd"): + argv[0] = argv[0] + ".cmd" try: - # Determine shell based on platform - if sys.platform == "win32": - # On Windows, use cmd.exe - shell_cmd = ["cmd", "/c", command] - else: - # On Unix-like systems, use sh - shell_cmd = ["sh", "-c", command] - - # Start subprocess with piped stdout/stderr - # stdin=DEVNULL prevents interactive dev servers from blocking on stdin - # On Windows, use CREATE_NO_WINDOW to prevent console window from flashing if sys.platform == "win32": self.process = subprocess.Popen( - shell_cmd, + argv, stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, @@ -330,25 +341,38 @@ class DevServerProcessManager: ) else: self.process = subprocess.Popen( - shell_cmd, + argv, stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, cwd=str(self.project_dir), ) - self._create_lock() - self.started_at = datetime.now() - self.status = "running" + self._command = command + self.started_at = datetime.utcnow() + self._detected_url = None - # Start output streaming task + # Create lock once we have a PID + self._create_lock() + + # Start output streaming + self.status = "running" self._output_task = asyncio.create_task(self._stream_output()) - return True, f"Dev server started with PID {self.process.pid}" + return True, "Dev server started" + + except FileNotFoundError: + self.status = "stopped" + self.process = None + self._remove_lock() + return False, f"Command not found: {argv[0]}" except Exception as e: - logger.exception("Failed to start dev server") + self.status = "stopped" + self.process = None + self._remove_lock() return False, f"Failed to start dev server: {e}" + async def stop(self) -> tuple[bool, str]: """ Stop the dev server (SIGTERM then SIGKILL if needed). @@ -487,8 +511,18 @@ def cleanup_orphaned_devserver_locks() -> int: if not project_path.exists(): continue - lock_file = project_path / ".devserver.lock" - if not lock_file.exists(): + # Check both legacy and new locations for lock files + from autocoder_paths import get_autocoder_dir + lock_locations = [ + project_path / ".devserver.lock", + get_autocoder_dir(project_path) / ".devserver.lock", + ] + lock_file = None + for candidate in lock_locations: + if candidate.exists(): + lock_file = candidate + break + if lock_file is None: continue try: