Improve Git server path validation and repository handling

- Enhance path safety checks with comprehensive security rules
- Add support for resolving repositories in single-directory and multi-repository modes
- Implement more robust error handling for repository path validation
- Improve error messages for repository initialization and path resolution
- Add additional checks to prevent access to system directories and unsafe paths

Author: sparesparrow

src/git/src/mcp_server_git/server.py

@@ -38,15 +38,46 @@ class PathValidator:
     def __init__(self, base_path: Path | str | None = None):
         self.base_path = Path(base_path).resolve() if base_path else None
 
+    def is_safe_path(self, path: Path) -> bool:
+        """Check if a path is safe to access.
+        
+        A path is considered safe if:
+        1. It's absolute
+        2. It doesn't contain symlinks that could bypass restrictions
+        3. If base_path is set, it must be within that directory
+        4. It's not a system directory (/etc, /usr, /bin, etc.)
+        """
+        try:
+            # Resolve to absolute path, following symlinks
+            resolved = path.resolve()
+            
+            # Check if it's a system directory
+            system_dirs = ["/etc", "/usr", "/bin", "/sbin", "/lib", "/sys", "/dev", "/proc"]
+            if any(str(resolved).startswith(sys_dir) for sys_dir in system_dirs):
+                return False
+                
+            # If base_path is set, ensure path is within it
+            if self.base_path:
+                return resolved.is_relative_to(self.base_path)
+                
+            # In unconfigured mode, apply additional restrictions
+            # Only allow paths under /home or custom project directories
+            safe_roots = ["/home", "/projects", "/repo"]
+            return any(str(resolved).startswith(root) for root in safe_roots)
+            
+        except (RuntimeError, OSError):
+            # RuntimeError can occur with circular symlinks
+            # OSError can occur with invalid paths
+            return False
+    
     def validate_path(self, path: str | Path) -> Path:
-        if not self.base_path:
-            raise ValueError("No base repository path configured")
+        """Validate and return a safe path."""
+        if '..' in Path(path).parts:
+            raise ValueError("Path traversal attempts not allowed")
 
-        # Normalize and compare canonical paths
         requested = Path(path).resolve()
 
-        # Check for path traversal and ensure path is within base directory
-        if not requested.is_relative_to(self.base_path):
+        if not self.is_safe_path(requested):
             raise ValueError(f"Path {requested} not in allowed scope")
 
         return requested
@@ -135,16 +166,47 @@ class GitTools(str, Enum):
 
 class GitServer:
     def __init__(self, base_path: Path | None = None):
-        self.path_validator = PathValidator(base_path)
+        self.base_path = Path(base_path).resolve() if base_path else None
+        self.path_validator = PathValidator(self.base_path)
         self.logger = logging.getLogger(__name__)
 
+    def resolve_repo_path(self, repo_path: str | Path) -> Path:
+        """Resolve repository path, handling both absolute and relative paths.
+        
+        In Docker environment, relative paths are resolved against base_path.
+        """
+        path = Path(repo_path)
+        if not path.is_absolute():
+            if not self.base_path:
+                raise ValueError("Base path must be set to use relative paths")
+            path = self.base_path / path
+        return path.resolve()
+
     def get_repo(self, repo_path: str) -> git.Repo:
-        """Get a Git repository with path validation."""
-        validated_path = self.path_validator.validate_path(repo_path)
+        """Get a Git repository with path and repository validation.
+        
+        Args:
+            repo_path: Path to the repository (absolute or relative to base_path)
+            
+        Returns:
+            git.Repo: The validated Git repository
+            
+        Raises:
+            ValueError: If the path is invalid or not a Git repository
+        """
+        resolved_path = self.resolve_repo_path(repo_path)
+        validated_path = self.path_validator.validate_path(resolved_path)
         try:
-            return git.Repo(validated_path)
+            repo = git.Repo(validated_path)
+            # Additional check to ensure the repo root is within allowed scope
+            repo_root = Path(repo.git_dir).parent.resolve()
+            if not self.path_validator.is_safe_path(repo_root):
+                raise ValueError(f"Repository root {repo_root} not in allowed scope")
+            return repo
         except git.InvalidGitRepositoryError:
-            raise ValueError(f"{validated_path} is not a valid Git repository")
+            raise ValueError(f"{validated_path} is not a Git repository")
+        except git.NoSuchPathError:
+            raise ValueError(f"Path {validated_path} does not exist")
 
     def git_status(self, repo: git.Repo) -> str:
         return repo.git.status()
@@ -253,24 +315,31 @@ async def serve(repository: Path | None) -> None:
 
     if repository is not None:
         repository = Path(repository).resolve()
-        try:
-            git.Repo(repository)
-            logger.info(f"Using repository at {repository}")
-        except git.InvalidGitRepositoryError:
-            logger.error(f"{repository} is not a valid Git repository")
+        if not repository.exists():
+            logger.error(f"Path {repository} does not exist")
+            return
+        if not repository.is_dir():
+            logger.error(f"Path {repository} is not a directory")
             return
+        logger.info(f"Using base directory at {repository}")
 
     server = Server("mcp-git")
-    git_server = GitServer(repository)
+    git_server = GitServer(base_path=repository)
 
     @server.list_tools()
     async def list_tools() -> list[Tool]:
         tools = []
         for tool in GitTools:
             schema = globals()[tool.value].schema()
-            if repository:  # Remove repo_path from schema when in single-repo mode
-                schema["properties"].pop("repo_path", None)
-                schema["required"] = [r for r in schema.get("required", []) if r != "repo_path"]
+            if repository:
+                # In single-repo mode with a parent directory,
+                # we still need repo_path but restrict it to subdirectories
+                if not git.repo.fun.is_git_dir(repository / ".git"):
+                    schema["properties"]["repo_path"]["description"] = "Path to Git repository (must be under the configured base directory)"
+                else:
+                    # If repository itself is a Git repo, remove repo_path as before
+                    schema["properties"].pop("repo_path", None)
+                    schema["required"] = [r for r in schema.get("required", []) if r != "repo_path"]
             tools.append(Tool(
                 name=tool.value,
                 description=TOOL_DESCRIPTIONS[tool],
@@ -281,20 +350,60 @@ async def list_tools() -> list[Tool]:
     @server.call_tool()
     async def call_tool(name: str, arguments: dict) -> list[TextContent]:
         try:
-            # Auto-use configured repository when specified
-            if repository:
+            # Auto-use configured repository only if it's actually a Git repository
+            if repository and git.repo.fun.is_git_dir(repository / ".git"):
+                arguments = arguments.copy()
                 arguments["repo_path"] = str(repository)
-                
+            elif repository:
+                # If repository is a parent dir, require and validate repo_path
+                if "repo_path" not in arguments:
+                    return [TextContent(
+                        type="text",
+                        text="Error: repository path not specified. When using --repository with a parent directory, you must specify which repository to operate on."
+                    )]
+                # No need to resolve here as get_repo will handle it
+                repo_path = arguments["repo_path"]
+                try:
+                    # Just validate it can be resolved and is within bounds
+                    resolved = git_server.resolve_repo_path(repo_path)
+                    if not resolved.is_relative_to(repository):
+                        return [TextContent(
+                            type="text",
+                            text=f"Error: repository path must be under the configured directory {repository}"
+                        )]
+                except ValueError as e:
+                    return [TextContent(
+                        type="text",
+                        text=f"Error: {str(e)}"
+                    )]
+
             # Handle git init separately
             if name == GitTools.INIT:
+                if "repo_path" not in arguments:
+                    return [TextContent(
+                        type="text",
+                        text="Error: repository path not specified"
+                    )]
                 result = git_server.git_init(arguments["repo_path"])
                 return [TextContent(
                     type="text",
                     text=result
                 )]
 
             # For all other commands, get a validated repo
-            repo = git_server.get_repo(arguments["repo_path"])
+            if "repo_path" not in arguments:
+                return [TextContent(
+                    type="text",
+                    text="Error: repository path not specified"
+                )]
+
+            try:
+                repo = git_server.get_repo(arguments["repo_path"])
+            except ValueError as e:
+                return [TextContent(
+                    type="text",
+                    text=f"Error: {str(e)}"
+                )]
 
             match name:
                 case GitTools.STATUS: