Resolve #604 Running mcp-server-git with uvx gives full disk access/--repository param is ignored

[sparesparrow]

Description

Security Enhancement: Robust Path Validation and Parent Directory Support

Key Improvements

1. Enhanced Path Security (server.py)

   • New PathValidator Class:

     • Blocks access to system directories (e.g. /etc, /usr, etc.)
     • Prevents path traversal (rejects paths containing ..)
     • Resolves symlinks and validates that paths are within safe roots
     • Supports resolving both absolute and relative paths, ensuring that when a base directory (via --repository) is configured, all paths are contained within it

   • Dual-Mode Validation:

     • Configured Mode:
       • When a valid Git repository is directly configured, tool calls automatically use the base repository and ignore any provided repo_path
       • When a parent directory is configured, every tool call must include an explicit repo_path that is validated against the base directory
     • Unconfigured Mode:
       • The server restricts repository paths to pre-approved safe roots

graph TD
    A[Client Request] --> B{--repository flag provided?}
    B -- Yes --> C[Configured Base Directory Provided]
    B -- No --> D[Require Explicit repo_path in Tool Call]

    C --> E{Is Configured Base a Git Repo?}
    E -- Yes --> F[Automatically use Base Repository]
    E -- No --> D

    D --> G[Validate Provided repo_path]
    G --> H{repo_path within Allowed Base?}
    H -- Yes --> I[Proceed with Operation]
    H -- No --> J[Reject Request with Error]

Loading

2. Server Initialization Improvements (__init__.py)

   • Removed the forced Git repository check during startup to support parent directories
   • Enhanced signal handling (SIGINT, SIGTERM) and graceful cleanup using an explicit event loop

3. Tool Access Control & Error Handling (server.py)

   • Each tool call now:
     • Validates the repository path using the new PathValidator
     • Resolves relative paths against the configured base directory
     • Provides clear, actionable error messages if the path is out-of-scope or invalid
   • Tools such as git_status, git_diff, git_create_branch, etc., dynamically adjust their input schema based on the current configuration mode

4. Documentation Updates (README.md)

   • Added a dedicated "Repository Path Security" section describing:
     • How operations are restricted to the configured repository (or subdirectories, if using a parent directory)
     • The enforcement of safe path practices (blocking ../ traversal, system directory protection)
   • Updated tool input details (e.g. renaming start_point to base_branch in branch creation)

5. Enhanced Testing (test_server.py)

   • New tests assert that:
     • Paths outside the allowed scope are correctly rejected
     • The server properly resolves a configured repository or a child repository when a parent directory is used
     • Git operations (branch creation, checkout, diff) succeed when given valid paths, and fail gracefully when not

Motivation and Context

This change is necessary to ensure that the Git MCP server is not vulnerable to malicious path traversal or unintended access to critical system directories. In addition to improving security:

• Input Validation: Only paths within the allowed boundaries are permitted.
• Flexibility: By introducing parent directory support, the server can now manage multiple repositories safely.
• Robust Logging: Enhanced error reporting allows users and administrators to diagnose issues more easily.

How Has This Been Tested?

1. Unit Tests
   • Extensive tests in test_server.py covering safe/unsafe path checks, repository validation, and tool operations
2. Integration Tests
   • Verified both single-repository mode and parent directory mode using UVX and LLM client calls
   • Tested against known system directories to ensure they are appropriately blocked
3. Security Tests
   • Confirmed that symlink resolution, relative-path prevention, and overall boundary enforcement work as expected

Breaking Changes

• Configuration Update (Parent Directory Mode):
  • When the --repository flag points to a parent directory (instead of a single Git repository), clients must provide an explicit repo_path with each tool call.
  • Existing configurations assuming a single repository might require adaptation.

Types of Changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (adds functionality for multi-repository support)
• Breaking change (may require configuration update for parent directory mode)
• Documentation update

Checklist

• I have read the MCP Protocol Documentation
• My changes follow MCP security best practices
• I have updated the server's README accordingly
• I have tested these changes with an LLM client
• My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling
• I have documented all environment variables and configuration options

Additional Context

• Defense-in-Depth Approach:

  1. Path normalization and resolution
  2. System directory deny list
  3. Symlink protection
  4. Dual-mode containment checks
  5. Copy-on-write argument handling

• Tool Dispatch Adjustments:
  Adjusts the schema and parameters on the fly based on whether the base directory is a single repository or a parent directory containing multiple repositories.

• Security Impact:
  This PR significantly improves the server's security posture by ensuring:

  1. Unauthorized repository access is prevented.
  2. System directories are blocked.
  3. Symlink vulnerabilities are reduced.
  4. Repository integrity is maintained through strict path validation.

Resolves #604

[olaservo]

Hi @sparesparrow thanks for opening this, fyi since a lot has changed since your last push I went ahead and:

• Resolved various conflicts
• Made sure the tests will run as part of CI and did some minor cleanup

[cliffhall]

This is an important set of changes, so I felt it was worthwhile to ask Copilot for a review of the code against the claims in the description and additional comments:

Claims & How the PR Addresses Them

1. Security Enhancement: Robust Path Validation and Parent Directory Support

• New PathValidator class:
  • Implemented in server.py.
  • Blocks access to system directories (/etc, /usr, etc.), prevents path traversal (..), resolves symlinks, and ensures paths are under allowed roots.
  • Validates both absolute and relative paths, crucial when a base directory (--repository) is configured.
• Dual-Mode Validation:
  • If a repository is configured and is a Git repo, all tool calls use that repo and ignore repo_path.
  • If configured as a parent directory, tool calls must include explicit repo_path validated against the base directory.
  • Otherwise, restricts repository paths to pre-approved safe roots.

Code evidence:

• The new PathValidator enforces all these rules.
• The server logic in server.py uses this validator for every operation.
• Tool schemas are dynamically adjusted based on configuration mode.

2. Server Initialization Improvements (__init__.py)

• Removed forced Git repo check at startup to allow for parent directory mode.
• Improved signal handling (SIGINT, SIGTERM) and cleanup with new event loop logic.

Code evidence:

• Startup now supports both a Git repo or a parent directory.
• Event loop and signal handling logic are present in the updated __init__.py.

3. Tool Access Control & Error Handling (server.py)

• Every tool call validates the repository path.
• Relative paths are resolved against the configured base directory.
• Clear error messages are provided for invalid/out-of-scope paths.
• Tool input schemas are dynamically adjusted.

Code evidence:

• All tool handlers go through the validation logic.
• Error messages are specific and actionable.
• Input schema for tools is changed based on configuration (single repo vs parent directory).

4. Documentation Updates (README.md)

• Added “Repository Path Security” section.
• Details on how operations are restricted to the configured repo or subdirectories.
• Describes blocked path traversal, system directory protection, and tool input changes (e.g., start_point → base_branch).

Code evidence:

• The README now contains a dedicated section explaining the security changes and usage.

5. Enhanced Testing (test_server.py)

• New/updated tests:
  • Paths outside allowed scope are rejected.
  • Server properly resolves repo/child repo when parent directory is used.
  • Valid git operations succeed, invalid ones fail gracefully.
  • Tests for both direct repo and parent directory modes.

Code evidence:

• Tests for path validation, repo resolution, and tool operations are present and cover the new modes/edge cases.

6. Dependency/Config Changes

• Updated dependencies (e.g., mcp>=1.10.1 in pyproject.toml and uv.lock) to support new features.

Code evidence:

• Dependency versions updated to support new features and security improvements.

Additional Points

• Defense-in-Depth:
  • Path normalization, system directory deny list, symlink protection, containment checks, and copy-on-write for arguments are all implemented.
• Tool Dispatch Adjustments:
  • Tool schema and parameters adapt based on server configuration.
• Security Impact:
  • Unauthorized repo access and system directory access are blocked.
  • Symlink vulnerabilities mitigated.
  • Strict path validation for repo integrity.

Summary

The PR fully implements the claimed security improvements, dual-mode operation, error handling, documentation, and testing. All claims in the description and comments are supported by concrete code changes, documentation updates, and expanded tests.

[cliffhall]

While I reviewed the code, I'm a Typescript-focused developer, so when I tried to run the unit tests locally, found that there was no documentation in the README about the requirements and procedures to do so.