Refactor dev server command execution and locking

Refactor dev server management to improve command execution and security checks. Introduce lock file handling and command validation enhancements.

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: