From 5a1eca7a83948eee757d28f43b6b2f0264902fbd Mon Sep 17 00:00:00 2001 From: mementorc Date: Thu, 21 May 2026 21:54:45 -0500 Subject: [PATCH] feat(github): add required_signatures branch-protection tools (#178) GitHub exposes commit-signature enforcement on a separate endpoint (.../protection/required_signatures), so it could not be set through github_update_branch_protection. Closes that gap by adding a dedicated enable/disable/get trio, mirroring the existing github_*_vulnerability_alerts pattern. New tools: - github_get_required_signatures(repo_owner, repo_name, branch) - github_enable_required_signatures(repo_owner, repo_name, branch) - github_disable_required_signatures(repo_owner, repo_name, branch) Endpoint: PUT/DELETE/GET /repos/{owner}/{repo}/branches/{branch}/protection/required_signatures - security.py: three async functions using github_client_context(). PUT returns 200+JSON (not 204 like vulnerability_alerts PUT); DELETE returns 204; GET returns 200 with {enabled, url} or 404 when branch protection is absent. - models.py: three Pydantic models with (repo_owner, repo_name, branch). - registry_github.py: ToolDefinition trio added after vulnerability_alerts. - api.py untouched (wildcard re-export of security.py covers new funcs). - README.md tool counts: 52 -> 55 and 51/22 -> 54/25. - tests/unit/github/test_github_required_signatures.py: 7 tests via AsyncMock+patch(github_client_context) covering get (enabled/disabled/ 404), enable (200/404), disable (204/404). Closes #178 --- README.md | 4 +- src/mcp_server_git/github/models.py | 24 ++ src/mcp_server_git/github/security.py | 119 ++++++++++ src/mcp_server_git/lean/registry_github.py | 27 +++ .../github/test_github_required_signatures.py | 215 ++++++++++++++++++ 5 files changed, 387 insertions(+), 2 deletions(-) create mode 100644 tests/unit/github/test_github_required_signatures.py diff --git a/README.md b/README.md index 7f4b6406..8627da3b 100644 --- a/README.md +++ b/README.md @@ -137,7 +137,7 @@ The server provides Azure DevOps integration for monitoring and analyzing Azure ### Lean MCP Interface (Context-Optimized) -The server provides an alternative **lean interface** that reduces context consumption by ~97% (from ~30k tokens to ~900 tokens). Instead of exposing all 52 tools upfront, it uses a 3-meta-tool pattern: +The server provides an alternative **lean interface** that reduces context consumption by ~97% (from ~30k tokens to ~900 tokens). Instead of exposing all 55 tools upfront, it uses a 3-meta-tool pattern: | Meta-Tool | Purpose | |-----------|---------| @@ -145,7 +145,7 @@ The server provides an alternative **lean interface** that reduces context consu | `get_tool_spec(tool_name)` | Get full schema for a specific tool on-demand | | `execute_tool(tool_name, params)` | Execute any tool dynamically | -**Tool Coverage**: All 51 tools remain accessible (26 git, 22 GitHub, 3 Azure DevOps). +**Tool Coverage**: All 54 tools remain accessible (26 git, 25 GitHub, 3 Azure DevOps). **Usage Example**: ```python diff --git a/src/mcp_server_git/github/models.py b/src/mcp_server_git/github/models.py index b120e5c2..41da750c 100644 --- a/src/mcp_server_git/github/models.py +++ b/src/mcp_server_git/github/models.py @@ -670,6 +670,30 @@ class GitHubDisableVulnerabilityAlerts(BaseModel): repo_name: str +class GitHubGetRequiredSignatures(BaseModel): + """Model for checking if required signatures are enabled on a protected branch.""" + + repo_owner: str + repo_name: str + branch: str + + +class GitHubEnableRequiredSignatures(BaseModel): + """Model for enabling required signatures on a protected branch.""" + + repo_owner: str + repo_name: str + branch: str + + +class GitHubDisableRequiredSignatures(BaseModel): + """Model for disabling required signatures on a protected branch.""" + + repo_owner: str + repo_name: str + branch: str + + class GitHubGetAutomatedSecurityFixes(BaseModel): """Model for checking if automated security fixes are enabled.""" diff --git a/src/mcp_server_git/github/security.py b/src/mcp_server_git/github/security.py index e9fe946c..ff03a147 100644 --- a/src/mcp_server_git/github/security.py +++ b/src/mcp_server_git/github/security.py @@ -1,7 +1,9 @@ """GitHub security operations - vulnerability alerts, security fixes, analysis.""" + from __future__ import annotations import logging from mcp_server_git.github.client import github_client_context + logger = logging.getLogger(__name__) @@ -115,6 +117,123 @@ async def github_disable_vulnerability_alerts( return f"❌ Error disabling vulnerability alerts: {str(e)}" +async def github_get_required_signatures( + repo_owner: str, + repo_name: str, + branch: str, +) -> str: + """Check if required signatures are enabled on a protected branch.""" + logger.debug( + f"🔍 Getting required signatures status for {repo_owner}/{repo_name}#{branch}" + ) + + try: + async with github_client_context() as client: + response = await client.get( + f"/repos/{repo_owner}/{repo_name}/branches/{branch}/protection/required_signatures" + ) + + if response.status == 200: + data = await response.json() + enabled = data.get("enabled", False) + return f"{'✅' if enabled else '❌'} Required signatures: {'enabled' if enabled else 'disabled'} on {repo_owner}/{repo_name}#{branch}" + elif response.status == 404: + return f"❌ Branch protection or branch not found: {repo_owner}/{repo_name}#{branch}" + else: + error_text = await response.text() + return f"❌ Failed to check required signatures: {response.status} - {error_text}" + + except ValueError as auth_error: + logger.error(f"Authentication error checking required signatures: {auth_error}") + return f"❌ {str(auth_error)}" + except ConnectionError as conn_error: + logger.error(f"Connection error checking required signatures: {conn_error}") + return f"❌ Network connection failed: {str(conn_error)}" + except Exception as e: + logger.error( + f"Unexpected error checking required signatures: {e}", exc_info=True + ) + return f"❌ Error checking required signatures: {str(e)}" + + +async def github_enable_required_signatures( + repo_owner: str, + repo_name: str, + branch: str, +) -> str: + """Enable required signatures on a protected branch.""" + logger.debug( + f"🚀 Enabling required signatures for {repo_owner}/{repo_name}#{branch}" + ) + + try: + async with github_client_context() as client: + response = await client.put( + f"/repos/{repo_owner}/{repo_name}/branches/{branch}/protection/required_signatures" + ) + + if response.status == 200: + logger.info( + f"✅ Enabled required signatures for {repo_owner}/{repo_name}#{branch}" + ) + return f"✅ Enabled required signatures on {repo_owner}/{repo_name}#{branch}" + else: + error_text = await response.text() + return f"❌ Failed to enable required signatures: {response.status} - {error_text}" + + except ValueError as auth_error: + logger.error(f"Authentication error enabling required signatures: {auth_error}") + return f"❌ {str(auth_error)}" + except ConnectionError as conn_error: + logger.error(f"Connection error enabling required signatures: {conn_error}") + return f"❌ Network connection failed: {str(conn_error)}" + except Exception as e: + logger.error( + f"Unexpected error enabling required signatures: {e}", exc_info=True + ) + return f"❌ Error enabling required signatures: {str(e)}" + + +async def github_disable_required_signatures( + repo_owner: str, + repo_name: str, + branch: str, +) -> str: + """Disable required signatures on a protected branch.""" + logger.debug( + f"🚀 Disabling required signatures for {repo_owner}/{repo_name}#{branch}" + ) + + try: + async with github_client_context() as client: + response = await client.delete( + f"/repos/{repo_owner}/{repo_name}/branches/{branch}/protection/required_signatures" + ) + + if response.status == 204: + logger.info( + f"✅ Disabled required signatures for {repo_owner}/{repo_name}#{branch}" + ) + return f"✅ Disabled required signatures on {repo_owner}/{repo_name}#{branch}" + else: + error_text = await response.text() + return f"❌ Failed to disable required signatures: {response.status} - {error_text}" + + except ValueError as auth_error: + logger.error( + f"Authentication error disabling required signatures: {auth_error}" + ) + return f"❌ {str(auth_error)}" + except ConnectionError as conn_error: + logger.error(f"Connection error disabling required signatures: {conn_error}") + return f"❌ Network connection failed: {str(conn_error)}" + except Exception as e: + logger.error( + f"Unexpected error disabling required signatures: {e}", exc_info=True + ) + return f"❌ Error disabling required signatures: {str(e)}" + + async def github_get_automated_security_fixes( repo_owner: str, repo_name: str, diff --git a/src/mcp_server_git/lean/registry_github.py b/src/mcp_server_git/lean/registry_github.py index d05a8d03..8f34957b 100644 --- a/src/mcp_server_git/lean/registry_github.py +++ b/src/mcp_server_git/lean/registry_github.py @@ -14,9 +14,11 @@ GitHubDeleteRelease, GitHubDeleteReleaseAsset, GitHubDisableAutomatedSecurityFixes, + GitHubDisableRequiredSignatures, GitHubDisableVulnerabilityAlerts, GitHubEditPRDescription, GitHubEnableAutomatedSecurityFixes, + GitHubEnableRequiredSignatures, GitHubEnableVulnerabilityAlerts, GitHubGetActionsPermissions, GitHubGetAutomatedSecurityFixes, @@ -29,6 +31,7 @@ GitHubGetPRFiles, GitHubGetPRStatus, GitHubGetRelease, + GitHubGetRequiredSignatures, GitHubGetRepoSettings, GitHubGetSecurityAnalysis, GitHubGetVulnerabilityAlerts, @@ -450,6 +453,30 @@ def _register_github_tools(interface: Any, github_service: Any): domain="github", complexity="focused", ), + ToolDefinition( + name="github_get_required_signatures", + implementation=github_ops.github_get_required_signatures, + description="Get required-signatures status for a protected branch", + schema=GitHubGetRequiredSignatures.model_json_schema(), + domain="github", + complexity="core", + ), + ToolDefinition( + name="github_enable_required_signatures", + implementation=github_ops.github_enable_required_signatures, + description="Enable required signatures on a protected branch", + schema=GitHubEnableRequiredSignatures.model_json_schema(), + domain="github", + complexity="focused", + ), + ToolDefinition( + name="github_disable_required_signatures", + implementation=github_ops.github_disable_required_signatures, + description="Disable required signatures on a protected branch", + schema=GitHubDisableRequiredSignatures.model_json_schema(), + domain="github", + complexity="focused", + ), ToolDefinition( name="github_get_automated_security_fixes", implementation=github_ops.github_get_automated_security_fixes, diff --git a/tests/unit/github/test_github_required_signatures.py b/tests/unit/github/test_github_required_signatures.py new file mode 100644 index 00000000..3153485c --- /dev/null +++ b/tests/unit/github/test_github_required_signatures.py @@ -0,0 +1,215 @@ +""" +Unit tests for GitHub required signatures functions. + +Tests the three required-signatures management functions: +- github_get_required_signatures +- github_enable_required_signatures +- github_disable_required_signatures +""" + +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from src.mcp_server_git.github.api import ( + github_disable_required_signatures, + github_enable_required_signatures, + github_get_required_signatures, +) + +_URL = "/repos/foo/bar/branches/main/protection/required_signatures" + + +class TestGithubRequiredSignatures: + """Test github_*_required_signatures functions.""" + + # ------------------------------------------------------------------ + # github_get_required_signatures + # ------------------------------------------------------------------ + + @pytest.mark.asyncio + async def test_get_returns_enabled_when_status_200_and_enabled_true(self): + """GET 200 with enabled=True returns an enabled confirmation.""" + mock_client = MagicMock() + + mock_response = AsyncMock() + mock_response.status = 200 + mock_response.json = AsyncMock(return_value={"enabled": True, "url": _URL}) + mock_client.get = AsyncMock(return_value=mock_response) + + with patch( + "src.mcp_server_git.github.security.github_client_context" + ) as mock_context: + mock_context.return_value.__aenter__.return_value = mock_client + + result = await github_get_required_signatures( + repo_owner="foo", + repo_name="bar", + branch="main", + ) + + assert "✅" in result + assert "enabled" in result + assert "foo/bar#main" in result + mock_client.get.assert_called_once_with(_URL) + + @pytest.mark.asyncio + async def test_get_returns_disabled_when_status_200_and_enabled_false(self): + """GET 200 with enabled=False returns a disabled indication.""" + mock_client = MagicMock() + + mock_response = AsyncMock() + mock_response.status = 200 + mock_response.json = AsyncMock(return_value={"enabled": False, "url": _URL}) + mock_client.get = AsyncMock(return_value=mock_response) + + with patch( + "src.mcp_server_git.github.security.github_client_context" + ) as mock_context: + mock_context.return_value.__aenter__.return_value = mock_client + + result = await github_get_required_signatures( + repo_owner="foo", + repo_name="bar", + branch="main", + ) + + assert "❌" in result + assert "disabled" in result + assert "foo/bar#main" in result + mock_client.get.assert_called_once_with(_URL) + + @pytest.mark.asyncio + async def test_get_returns_not_found_when_status_404(self): + """GET 404 reports that branch protection or branch was not found.""" + mock_client = MagicMock() + + mock_response = AsyncMock() + mock_response.status = 404 + mock_client.get = AsyncMock(return_value=mock_response) + + with patch( + "src.mcp_server_git.github.security.github_client_context" + ) as mock_context: + mock_context.return_value.__aenter__.return_value = mock_client + + result = await github_get_required_signatures( + repo_owner="foo", + repo_name="bar", + branch="main", + ) + + assert "❌" in result + assert "Branch protection or branch not found" in result + assert "foo/bar#main" in result + mock_client.get.assert_called_once_with(_URL) + + # ------------------------------------------------------------------ + # github_enable_required_signatures + # ------------------------------------------------------------------ + + @pytest.mark.asyncio + async def test_enable_returns_success_when_status_200(self): + """PUT 200 returns a success confirmation.""" + mock_client = MagicMock() + + mock_response = AsyncMock() + mock_response.status = 200 + mock_client.put = AsyncMock(return_value=mock_response) + + with patch( + "src.mcp_server_git.github.security.github_client_context" + ) as mock_context: + mock_context.return_value.__aenter__.return_value = mock_client + + result = await github_enable_required_signatures( + repo_owner="foo", + repo_name="bar", + branch="main", + ) + + assert "✅" in result + assert "Enabled required signatures" in result + assert "foo/bar#main" in result + mock_client.put.assert_called_once_with(_URL) + + @pytest.mark.asyncio + async def test_enable_returns_error_when_status_404(self): + """PUT non-200 (e.g. branch not protected) returns a failure message.""" + mock_client = MagicMock() + + mock_response = AsyncMock() + mock_response.status = 404 + mock_response.text = AsyncMock(return_value="Not Found") + mock_client.put = AsyncMock(return_value=mock_response) + + with patch( + "src.mcp_server_git.github.security.github_client_context" + ) as mock_context: + mock_context.return_value.__aenter__.return_value = mock_client + + result = await github_enable_required_signatures( + repo_owner="foo", + repo_name="bar", + branch="main", + ) + + assert "❌" in result + assert "Failed to enable required signatures" in result + assert "404" in result + mock_client.put.assert_called_once_with(_URL) + + # ------------------------------------------------------------------ + # github_disable_required_signatures + # ------------------------------------------------------------------ + + @pytest.mark.asyncio + async def test_disable_returns_success_when_status_204(self): + """DELETE 204 returns a success confirmation.""" + mock_client = MagicMock() + + mock_response = AsyncMock() + mock_response.status = 204 + mock_client.delete = AsyncMock(return_value=mock_response) + + with patch( + "src.mcp_server_git.github.security.github_client_context" + ) as mock_context: + mock_context.return_value.__aenter__.return_value = mock_client + + result = await github_disable_required_signatures( + repo_owner="foo", + repo_name="bar", + branch="main", + ) + + assert "✅" in result + assert "Disabled required signatures" in result + assert "foo/bar#main" in result + mock_client.delete.assert_called_once_with(_URL) + + @pytest.mark.asyncio + async def test_disable_returns_error_when_status_404(self): + """DELETE non-204 returns a failure message.""" + mock_client = MagicMock() + + mock_response = AsyncMock() + mock_response.status = 404 + mock_response.text = AsyncMock(return_value="Not Found") + mock_client.delete = AsyncMock(return_value=mock_response) + + with patch( + "src.mcp_server_git.github.security.github_client_context" + ) as mock_context: + mock_context.return_value.__aenter__.return_value = mock_client + + result = await github_disable_required_signatures( + repo_owner="foo", + repo_name="bar", + branch="main", + ) + + assert "❌" in result + assert "Failed to disable required signatures" in result + assert "404" in result + mock_client.delete.assert_called_once_with(_URL)