Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sparesparrow
Copy link

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
  1. 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
  2. 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
  3. 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)
  4. 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Running mcp-server-git with uvx gives full disk access/--repository param is ignored
1 participant