**feat(git-ops): enhance MCP server with Git provider updates and SSRF protection**
- Added `mcp-git-ops` service to `docker-compose.dev.yml` with health checks and configurations. - Integrated SSRF protection in repository URL validation for enhanced security. - Expanded `pyproject.toml` mypy settings and adjusted code to meet stricter type checking. - Improved workspace management and GitWrapper operations with error handling refinements. - Updated input validation, branching, and repository operations to align with new error structure. - Shut down thread pool executor gracefully during server cleanup.
This commit is contained in:
@@ -52,6 +52,21 @@ from models import (
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def sanitize_url_for_logging(url: str) -> str:
|
||||
"""
|
||||
Remove any credentials from a URL before logging.
|
||||
|
||||
Handles URLs like:
|
||||
- https://token@github.com/owner/repo.git
|
||||
- https://user:password@github.com/owner/repo.git
|
||||
- git@github.com:owner/repo.git (unchanged, no credentials)
|
||||
"""
|
||||
# Pattern to match https://[credentials@]host/path
|
||||
sanitized = re.sub(r"(https?://)([^@]+@)", r"\1***@", url)
|
||||
return sanitized
|
||||
|
||||
|
||||
# Thread pool for blocking git operations
|
||||
_executor: ThreadPoolExecutor | None = None
|
||||
|
||||
@@ -81,7 +96,7 @@ class GitWrapper:
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
workspace_path: Path,
|
||||
workspace_path: Path | str,
|
||||
settings: Settings | None = None,
|
||||
) -> None:
|
||||
"""
|
||||
@@ -91,7 +106,9 @@ class GitWrapper:
|
||||
workspace_path: Path to the git workspace
|
||||
settings: Optional settings override
|
||||
"""
|
||||
self.workspace_path = workspace_path
|
||||
self.workspace_path = (
|
||||
Path(workspace_path) if isinstance(workspace_path, str) else workspace_path
|
||||
)
|
||||
self.settings = settings or get_settings()
|
||||
self._repo: GitRepo | None = None
|
||||
|
||||
@@ -175,8 +192,10 @@ class GitWrapper:
|
||||
)
|
||||
|
||||
except GitCommandError as e:
|
||||
logger.error(f"Clone failed: {e}")
|
||||
raise CloneError(repo_url, str(e))
|
||||
# Sanitize URLs in error messages to prevent credential leakage
|
||||
error_msg = sanitize_url_for_logging(str(e))
|
||||
logger.error(f"Clone failed: {error_msg}")
|
||||
raise CloneError(sanitize_url_for_logging(repo_url), error_msg)
|
||||
|
||||
return await run_in_executor(_do_clone)
|
||||
|
||||
@@ -200,9 +219,10 @@ class GitWrapper:
|
||||
staged = []
|
||||
for diff in repo.index.diff("HEAD"):
|
||||
change_type = self._diff_to_change_type(diff.change_type)
|
||||
path = diff.b_path or diff.a_path or ""
|
||||
staged.append(
|
||||
FileChange(
|
||||
path=diff.b_path or diff.a_path,
|
||||
path=path,
|
||||
change_type=change_type,
|
||||
old_path=diff.a_path if diff.renamed else None,
|
||||
).to_dict()
|
||||
@@ -212,9 +232,10 @@ class GitWrapper:
|
||||
unstaged = []
|
||||
for diff in repo.index.diff(None):
|
||||
change_type = self._diff_to_change_type(diff.change_type)
|
||||
path = diff.b_path or diff.a_path or ""
|
||||
unstaged.append(
|
||||
FileChange(
|
||||
path=diff.b_path or diff.a_path,
|
||||
path=path,
|
||||
change_type=change_type,
|
||||
).to_dict()
|
||||
)
|
||||
@@ -228,10 +249,18 @@ class GitWrapper:
|
||||
tracking = repo.active_branch.tracking_branch()
|
||||
if tracking:
|
||||
ahead = len(
|
||||
list(repo.iter_commits(f"{tracking.name}..{repo.active_branch.name}"))
|
||||
list(
|
||||
repo.iter_commits(
|
||||
f"{tracking.name}..{repo.active_branch.name}"
|
||||
)
|
||||
)
|
||||
)
|
||||
behind = len(
|
||||
list(repo.iter_commits(f"{repo.active_branch.name}..{tracking.name}"))
|
||||
list(
|
||||
repo.iter_commits(
|
||||
f"{repo.active_branch.name}..{tracking.name}"
|
||||
)
|
||||
)
|
||||
)
|
||||
except Exception:
|
||||
pass # No tracking branch
|
||||
@@ -361,6 +390,12 @@ class GitWrapper:
|
||||
local_branches = []
|
||||
for branch in repo.branches:
|
||||
tracking = branch.tracking_branch()
|
||||
msg = branch.commit.message
|
||||
commit_msg = (
|
||||
msg.decode("utf-8", errors="replace")
|
||||
if isinstance(msg, bytes)
|
||||
else msg
|
||||
).split("\n")[0]
|
||||
local_branches.append(
|
||||
BranchInfo(
|
||||
name=branch.name,
|
||||
@@ -368,7 +403,7 @@ class GitWrapper:
|
||||
is_remote=False,
|
||||
tracking_branch=tracking.name if tracking else None,
|
||||
commit_sha=branch.commit.hexsha,
|
||||
commit_message=branch.commit.message.split("\n")[0],
|
||||
commit_message=commit_msg,
|
||||
).to_dict()
|
||||
)
|
||||
|
||||
@@ -379,13 +414,19 @@ class GitWrapper:
|
||||
# Skip HEAD refs
|
||||
if ref.name.endswith("/HEAD"):
|
||||
continue
|
||||
msg = ref.commit.message
|
||||
commit_msg = (
|
||||
msg.decode("utf-8", errors="replace")
|
||||
if isinstance(msg, bytes)
|
||||
else msg
|
||||
).split("\n")[0]
|
||||
remote_branches.append(
|
||||
BranchInfo(
|
||||
name=ref.name,
|
||||
is_current=False,
|
||||
is_remote=True,
|
||||
commit_sha=ref.commit.hexsha,
|
||||
commit_message=ref.commit.message.split("\n")[0],
|
||||
commit_message=commit_msg,
|
||||
).to_dict()
|
||||
)
|
||||
|
||||
@@ -485,7 +526,11 @@ class GitWrapper:
|
||||
repo.git.add("-A")
|
||||
|
||||
# Check if there's anything to commit
|
||||
if not allow_empty and not repo.index.diff("HEAD") and not repo.untracked_files:
|
||||
if (
|
||||
not allow_empty
|
||||
and not repo.index.diff("HEAD")
|
||||
and not repo.untracked_files
|
||||
):
|
||||
raise CommitError("Nothing to commit")
|
||||
|
||||
# Build author
|
||||
@@ -613,7 +658,7 @@ class GitWrapper:
|
||||
|
||||
try:
|
||||
# Build push info
|
||||
push_info_list = []
|
||||
push_info_list: list[Any] = []
|
||||
|
||||
if remote not in [r.name for r in repo.remotes]:
|
||||
raise PushError(push_branch, f"Remote not found: {remote}")
|
||||
@@ -716,9 +761,7 @@ class GitWrapper:
|
||||
repo.git.pull(remote, pull_branch)
|
||||
|
||||
# Count new commits
|
||||
commits_received = len(
|
||||
list(repo.iter_commits(f"{head_before}..HEAD"))
|
||||
)
|
||||
commits_received = len(list(repo.iter_commits(f"{head_before}..HEAD")))
|
||||
|
||||
# Check if fast-forward
|
||||
fast_forward = commits_received > 0 and not repo.head.commit.parents
|
||||
@@ -733,10 +776,9 @@ class GitWrapper:
|
||||
except GitCommandError as e:
|
||||
error_msg = str(e)
|
||||
if "conflict" in error_msg.lower():
|
||||
# Get conflicting files
|
||||
# Get conflicting files - keys are paths directly
|
||||
conflicts = [
|
||||
item.a_path
|
||||
for item in repo.index.unmerged_blobs().keys()
|
||||
str(path) for path in repo.index.unmerged_blobs().keys()
|
||||
]
|
||||
raise MergeConflictError(conflicts)
|
||||
raise PullError(pull_branch, error_msg)
|
||||
@@ -823,7 +865,7 @@ class GitWrapper:
|
||||
continue
|
||||
|
||||
change_type = self._diff_to_change_type(diff.change_type)
|
||||
path = diff.b_path or diff.a_path
|
||||
path = diff.b_path or diff.a_path or ""
|
||||
|
||||
# Parse hunks from patch
|
||||
hunks = []
|
||||
@@ -831,7 +873,13 @@ class GitWrapper:
|
||||
deletions = 0
|
||||
|
||||
if diff.diff:
|
||||
patch_text = diff.diff.decode("utf-8", errors="replace")
|
||||
# Handle both bytes and str
|
||||
raw_diff = diff.diff
|
||||
patch_text = (
|
||||
raw_diff.decode("utf-8", errors="replace")
|
||||
if isinstance(raw_diff, bytes)
|
||||
else raw_diff
|
||||
)
|
||||
# Parse hunks (simplified)
|
||||
for line in patch_text.split("\n"):
|
||||
if line.startswith("+") and not line.startswith("+++"):
|
||||
@@ -921,18 +969,25 @@ class GitWrapper:
|
||||
iterator = repo.iter_commits(**kwargs)
|
||||
|
||||
for commit in iterator:
|
||||
# Handle message that can be bytes
|
||||
msg = commit.message
|
||||
message_str = (
|
||||
msg.decode("utf-8", errors="replace")
|
||||
if isinstance(msg, bytes)
|
||||
else msg
|
||||
)
|
||||
commits.append(
|
||||
CommitInfo(
|
||||
sha=commit.hexsha,
|
||||
short_sha=commit.hexsha[:7],
|
||||
message=commit.message,
|
||||
author_name=commit.author.name,
|
||||
author_email=commit.author.email,
|
||||
message=message_str,
|
||||
author_name=commit.author.name or "Unknown",
|
||||
author_email=commit.author.email or "",
|
||||
authored_date=datetime.fromtimestamp(
|
||||
commit.authored_date, tz=UTC
|
||||
),
|
||||
committer_name=commit.committer.name,
|
||||
committer_email=commit.committer.email,
|
||||
committer_name=commit.committer.name or "Unknown",
|
||||
committer_email=commit.committer.email or "",
|
||||
committed_date=datetime.fromtimestamp(
|
||||
commit.committed_date, tz=UTC
|
||||
),
|
||||
@@ -1052,8 +1107,10 @@ class GitWrapper:
|
||||
|
||||
# Utility methods
|
||||
|
||||
def _diff_to_change_type(self, change_type: str) -> FileChangeType:
|
||||
def _diff_to_change_type(self, change_type: str | None) -> FileChangeType:
|
||||
"""Convert GitPython change type to our enum."""
|
||||
if change_type is None:
|
||||
return FileChangeType.MODIFIED
|
||||
mapping = {
|
||||
"A": FileChangeType.ADDED,
|
||||
"M": FileChangeType.MODIFIED,
|
||||
@@ -1105,7 +1162,8 @@ class GitWrapper:
|
||||
try:
|
||||
cr = repo.config_reader()
|
||||
section, option = key.rsplit(".", 1)
|
||||
return cr.get_value(section, option)
|
||||
value = cr.get_value(section, option)
|
||||
return str(value) if value is not None else None
|
||||
except Exception:
|
||||
return None
|
||||
|
||||
|
||||
Reference in New Issue
Block a user