Skip to content

Validate GitHub API parameters to prevent file paths in repo_owner/repo_name #105

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.

Sign up for GitHub

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

Closed
Copilot wants to merge 5 commits into from
Closed

Validate GitHub API parameters to prevent file paths in repo_owner/repo_name #105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
43e0865
Initial plan
Copilot Jan 10, 2026
f51a2b2
Add validation to prevent GitHub operations from using repository paths
Copilot Jan 10, 2026
1192d66
Add comprehensive tests for GitHub parameter validation
Copilot Jan 10, 2026
d28be3a
Address code review feedback
Copilot Jan 10, 2026
6ebda22
Refactor validation logic to reduce code duplication
Copilot Jan 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
157 changes: 156 additions & 1 deletion src/mcp_server_git/applications/server_application.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import asyncio
import logging
import os
import signal
import sys
from collections.abc import AsyncIterator
Expand Down Expand Up @@ -1500,6 +1501,63 @@ class MCPResponse:

logger.info("MCP tools registered successfully")

def _looks_like_file_path(self, value: str) -> bool:
"""
Check if a string looks like a file path rather than a GitHub identifier.

Args:
value: String to check

Returns:
True if the value appears to be a file path, False otherwise
"""
# Check for path separators (forward or back slash)
if "/" in value or "\\" in value:
return True

# Check if it's an absolute path
# Note: This is a conservative check. While it might flag edge cases like
# "C:" on Windows, GitHub itself doesn't allow colons in repository names,
# so this provides an extra layer of safety.
if os.path.isabs(value):
return True

return False

def _validate_github_params(self, repo_owner: str, repo_name: str) -> None:
"""
Validate GitHub API parameters to ensure they're not file paths.

GitHub API operations use repo_owner and repo_name (e.g., "MementoRC", "mcp-git"),
NOT local file system paths. This validation prevents confusion between:
- Local git operations: use repository paths (e.g., "/home/user/repo")
- GitHub API operations: use owner/name pairs (e.g., "owner", "repo")

Args:
repo_owner: GitHub repository owner/organization name
repo_name: GitHub repository name

Raises:
ValueError: If parameters look like file paths instead of GitHub identifiers
"""
# Check if repo_owner looks like a file path
if self._looks_like_file_path(repo_owner):
raise ValueError(
f"Invalid repo_owner: '{repo_owner}' appears to be a file path. "
f"GitHub API operations require repository owner/name parameters, not local paths. "
f"Use 'repo_owner' (e.g., 'MementoRC') and 'repo_name' (e.g., 'mcp-git'), "
f"not the bound '--repository' path."
)

# Check if repo_name looks like a file path
if self._looks_like_file_path(repo_name):
raise ValueError(
f"Invalid repo_name: '{repo_name}' appears to be a file path. "
f"GitHub API operations require repository owner/name parameters, not local paths. "
f"Use 'repo_owner' (e.g., 'MementoRC') and 'repo_name' (e.g., 'mcp-git'), "
f"not the bound '--repository' path."
)

async def _execute_tool_operation(self, name: str, arguments: dict):
"""Execute the actual tool logic without middleware."""
# COMPREHENSIVE INTEGRATED LOGGING
Expand Down Expand Up @@ -1537,7 +1595,8 @@ async def _execute_tool_operation(self, name: str, arguments: dict):
)
from ..utils.git_import import Repo

# Get repository path from arguments
# Get repository path from arguments for LOCAL git operations ONLY
# NOTE: This path is used ONLY for git operations, NOT for GitHub API operations
default_repo_path: str = (
str(self.config.repository_path) if self.config.repository_path else "."
)
Expand Down Expand Up @@ -1670,9 +1729,15 @@ async def _execute_tool_operation(self, name: str, arguments: dict):
elif name == GitTools.REMOTE_GET_URL:
result = git_remote_get_url(repo, arguments["name"])
# GitHub Tools (don't require a Repo object)
# NOTE: GitHub API operations use repo_owner/repo_name, NOT local file paths
elif name == GitHubTools.CREATE_ISSUE:
from ..github.api import github_create_issue

# Validate that repo_owner/repo_name are not file paths
self._validate_github_params(
arguments["repo_owner"], arguments["repo_name"]
)

result = await github_create_issue(
repo_owner=arguments["repo_owner"],
repo_name=arguments["repo_name"],
Expand All @@ -1685,6 +1750,11 @@ async def _execute_tool_operation(self, name: str, arguments: dict):
elif name == GitHubTools.LIST_ISSUES:
from ..github.api import github_list_issues

# Validate that repo_owner/repo_name are not file paths
self._validate_github_params(
arguments["repo_owner"], arguments["repo_name"]
)

result = await github_list_issues(
repo_owner=arguments["repo_owner"],
repo_name=arguments["repo_name"],
Expand All @@ -1703,6 +1773,11 @@ async def _execute_tool_operation(self, name: str, arguments: dict):
elif name == GitHubTools.UPDATE_ISSUE:
from ..github.api import github_update_issue

# Validate that repo_owner/repo_name are not file paths
self._validate_github_params(
arguments["repo_owner"], arguments["repo_name"]
)

result = await github_update_issue(
repo_owner=arguments["repo_owner"],
repo_name=arguments["repo_name"],
Expand All @@ -1717,6 +1792,11 @@ async def _execute_tool_operation(self, name: str, arguments: dict):
elif name == GitHubTools.GET_PR_CHECKS:
from ..github.api import github_get_pr_checks

# Validate that repo_owner/repo_name are not file paths
self._validate_github_params(
arguments["repo_owner"], arguments["repo_name"]
)

result = await github_get_pr_checks(
repo_owner=arguments["repo_owner"],
repo_name=arguments["repo_name"],
Expand All @@ -1727,6 +1807,11 @@ async def _execute_tool_operation(self, name: str, arguments: dict):
elif name == GitHubTools.GET_PR_DETAILS:
from ..github.api import github_get_pr_details

# Validate that repo_owner/repo_name are not file paths
self._validate_github_params(
arguments["repo_owner"], arguments["repo_name"]
)

result = await github_get_pr_details(
repo_owner=arguments["repo_owner"],
repo_name=arguments["repo_name"],
Expand All @@ -1737,6 +1822,11 @@ async def _execute_tool_operation(self, name: str, arguments: dict):
elif name == GitHubTools.LIST_PULL_REQUESTS:
from ..github.api import github_list_pull_requests

# Validate that repo_owner/repo_name are not file paths
self._validate_github_params(
arguments["repo_owner"], arguments["repo_name"]
)

result = await github_list_pull_requests(
repo_owner=arguments["repo_owner"],
repo_name=arguments["repo_name"],
Expand All @@ -1751,6 +1841,11 @@ async def _execute_tool_operation(self, name: str, arguments: dict):
elif name == GitHubTools.GET_PR_STATUS:
from ..github.api import github_get_pr_status

# Validate that repo_owner/repo_name are not file paths
self._validate_github_params(
arguments["repo_owner"], arguments["repo_name"]
)

result = await github_get_pr_status(
repo_owner=arguments["repo_owner"],
repo_name=arguments["repo_name"],
Expand All @@ -1759,6 +1854,11 @@ async def _execute_tool_operation(self, name: str, arguments: dict):
elif name == GitHubTools.GET_PR_FILES:
from ..github.api import github_get_pr_files

# Validate that repo_owner/repo_name are not file paths
self._validate_github_params(
arguments["repo_owner"], arguments["repo_name"]
)

result = await github_get_pr_files(
repo_owner=arguments["repo_owner"],
repo_name=arguments["repo_name"],
Expand All @@ -1770,6 +1870,11 @@ async def _execute_tool_operation(self, name: str, arguments: dict):
elif name == GitHubTools.EDIT_PR_DESCRIPTION:
from ..github.api import github_edit_pr_description

# Validate that repo_owner/repo_name are not file paths
self._validate_github_params(
arguments["repo_owner"], arguments["repo_name"]
)

result = await github_edit_pr_description(
repo_owner=arguments["repo_owner"],
repo_name=arguments["repo_name"],
Expand All @@ -1779,6 +1884,11 @@ async def _execute_tool_operation(self, name: str, arguments: dict):
elif name == GitHubTools.GET_WORKFLOW_RUN:
from ..github.api import github_get_workflow_run

# Validate that repo_owner/repo_name are not file paths
self._validate_github_params(
arguments["repo_owner"], arguments["repo_name"]
)

result = await github_get_workflow_run(
repo_owner=arguments["repo_owner"],
repo_name=arguments["repo_name"],
Expand All @@ -1788,6 +1898,11 @@ async def _execute_tool_operation(self, name: str, arguments: dict):
elif name == GitHubTools.LIST_WORKFLOW_RUNS:
from ..github.api import github_list_workflow_runs

# Validate that repo_owner/repo_name are not file paths
self._validate_github_params(
arguments["repo_owner"], arguments["repo_name"]
)

result = await github_list_workflow_runs(
repo_owner=arguments["repo_owner"],
repo_name=arguments["repo_name"],
Expand All @@ -1811,6 +1926,11 @@ async def _execute_tool_operation(self, name: str, arguments: dict):
if "repo_owner" not in arguments or "repo_name" not in arguments:
result = "Error: repo_owner and repo_name are required arguments"
else:
# Validate that repo_owner/repo_name are not file paths
self._validate_github_params(
arguments["repo_owner"], arguments["repo_name"]
)

result = await github_await_workflow_completion(
repo_owner=arguments["repo_owner"],
repo_name=arguments["repo_name"],
Expand All @@ -1821,6 +1941,11 @@ async def _execute_tool_operation(self, name: str, arguments: dict):
elif name == GitHubTools.CREATE_PR:
from ..github.api import github_create_pr

# Validate that repo_owner/repo_name are not file paths
self._validate_github_params(
arguments["repo_owner"], arguments["repo_name"]
)

result = await github_create_pr(
repo_owner=arguments["repo_owner"],
repo_name=arguments["repo_name"],
Expand All @@ -1833,6 +1958,11 @@ async def _execute_tool_operation(self, name: str, arguments: dict):
elif name == GitHubTools.MERGE_PR:
from ..github.api import github_merge_pr

# Validate that repo_owner/repo_name are not file paths
self._validate_github_params(
arguments["repo_owner"], arguments["repo_name"]
)

result = await github_merge_pr(
repo_owner=arguments["repo_owner"],
repo_name=arguments["repo_name"],
Expand All @@ -1844,6 +1974,11 @@ async def _execute_tool_operation(self, name: str, arguments: dict):
elif name == GitHubTools.ADD_PR_COMMENT:
from ..github.api import github_add_pr_comment

# Validate that repo_owner/repo_name are not file paths
self._validate_github_params(
arguments["repo_owner"], arguments["repo_name"]
)

result = await github_add_pr_comment(
repo_owner=arguments["repo_owner"],
repo_name=arguments["repo_name"],
Expand All @@ -1853,6 +1988,11 @@ async def _execute_tool_operation(self, name: str, arguments: dict):
elif name == GitHubTools.CLOSE_PR:
from ..github.api import github_close_pr

# Validate that repo_owner/repo_name are not file paths
self._validate_github_params(
arguments["repo_owner"], arguments["repo_name"]
)

result = await github_close_pr(
repo_owner=arguments["repo_owner"],
repo_name=arguments["repo_name"],
Expand All @@ -1861,6 +2001,11 @@ async def _execute_tool_operation(self, name: str, arguments: dict):
elif name == GitHubTools.REOPEN_PR:
from ..github.api import github_reopen_pr

# Validate that repo_owner/repo_name are not file paths
self._validate_github_params(
arguments["repo_owner"], arguments["repo_name"]
)

result = await github_reopen_pr(
repo_owner=arguments["repo_owner"],
repo_name=arguments["repo_name"],
Expand All @@ -1869,6 +2014,11 @@ async def _execute_tool_operation(self, name: str, arguments: dict):
elif name == GitHubTools.UPDATE_PR:
from ..github.api import github_update_pr

# Validate that repo_owner/repo_name are not file paths
self._validate_github_params(
arguments["repo_owner"], arguments["repo_name"]
)

result = await github_update_pr(
repo_owner=arguments["repo_owner"],
repo_name=arguments["repo_name"],
Expand All @@ -1881,6 +2031,11 @@ async def _execute_tool_operation(self, name: str, arguments: dict):
elif name == GitHubTools.GET_FAILING_JOBS:
from ..github.api import github_get_failing_jobs

# Validate that repo_owner/repo_name are not file paths
self._validate_github_params(
arguments["repo_owner"], arguments["repo_name"]
)

result = await github_get_failing_jobs(
repo_owner=arguments["repo_owner"],
repo_name=arguments["repo_name"],
Expand Down
Loading
Loading