From 3793ac965c8a83962771b4b50e767f38a1310c64 Mon Sep 17 00:00:00 2001 From: smadi0x86 Date: Tue, 8 Apr 2025 16:51:13 +0300 Subject: [PATCH 1/2] feat: implement Dependabot security update tools This PR implements tools for managing Dependabot security updates in repositories. However, during implementation, we encountered an issue with GitHub's API behavior where public repositories should have Dependabot alerts enabled by default, but the API still requires explicit enabling of vulnerability alerts. This creates a confusing user experience. We've documented this issue in the code for future investigation. --- pkg/github/security.go | 236 ++++++++++++++++++ pkg/github/security_test.go | 470 ++++++++++++++++++++++++++++++++++++ pkg/github/server.go | 12 + 3 files changed, 718 insertions(+) create mode 100644 pkg/github/security.go create mode 100644 pkg/github/security_test.go diff --git a/pkg/github/security.go b/pkg/github/security.go new file mode 100644 index 00000000..0446be3e --- /dev/null +++ b/pkg/github/security.go @@ -0,0 +1,236 @@ +package github + +import ( + "context" + "encoding/json" + "fmt" + + "github.com/github/github-mcp-server/pkg/translations" + "github.com/google/go-github/v69/github" + "github.com/mark3labs/mcp-go/mcp" + "github.com/mark3labs/mcp-go/server" +) + +// SecurityAndAnalysis represents the security and analysis settings for a repository +type SecurityAndAnalysis struct { + AdvancedSecurity struct { + Status string `json:"status"` + } `json:"advanced_security"` + SecretScanning struct { + Status string `json:"status"` + } `json:"secret_scanning"` + SecretScanningPushProtection struct { + Status string `json:"status"` + } `json:"secret_scanning_push_protection"` +} + +// GetSecuritySettings retrieves security settings for a repository +func GetSecuritySettings(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("get_security_settings", + mcp.WithDescription(t("TOOL_GET_SECURITY_SETTINGS_DESCRIPTION", "Get security settings for a repository")), + mcp.WithString("owner", + mcp.Required(), + mcp.Description(t("PARAM_OWNER_DESCRIPTION", "Repository owner")), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description(t("PARAM_REPO_DESCRIPTION", "Repository name")), + ), + ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + owner, ok := request.Params.Arguments["owner"].(string) + if !ok { + return nil, fmt.Errorf("missing required parameter: owner") + } + + repo, ok := request.Params.Arguments["repo"].(string) + if !ok { + return nil, fmt.Errorf("missing required parameter: repo") + } + + repository, _, err := client.Repositories.Get(ctx, owner, repo) + if err != nil { + return nil, fmt.Errorf("failed to get repository settings: %w", err) + } + + response, err := json.Marshal(repository.SecurityAndAnalysis) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(response)), nil + } +} + +// UpdateSecuritySettings updates security settings for a repository +func UpdateSecuritySettings(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("update_security_settings", + mcp.WithDescription(t("TOOL_UPDATE_SECURITY_SETTINGS_DESCRIPTION", "Update security settings for a repository")), + mcp.WithString("owner", + mcp.Required(), + mcp.Description(t("PARAM_OWNER_DESCRIPTION", "Repository owner")), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description(t("PARAM_REPO_DESCRIPTION", "Repository name")), + ), + mcp.WithObject("settings", + mcp.Required(), + mcp.Description(t("PARAM_SETTINGS_DESCRIPTION", "Security settings to update")), + ), + ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + owner, ok := request.Params.Arguments["owner"].(string) + if !ok { + return nil, fmt.Errorf("missing required parameter: owner") + } + + repo, ok := request.Params.Arguments["repo"].(string) + if !ok { + return nil, fmt.Errorf("missing required parameter: repo") + } + + settings, ok := request.Params.Arguments["settings"].(map[string]interface{}) + if !ok { + return nil, fmt.Errorf("missing required parameter: settings") + } + + // Get current repository settings + repository, _, err := client.Repositories.Get(ctx, owner, repo) + if err != nil { + return nil, fmt.Errorf("failed to get repository settings: %w", err) + } + + // Initialize security settings if nil + if repository.SecurityAndAnalysis == nil { + repository.SecurityAndAnalysis = &github.SecurityAndAnalysis{} + } + + // Update vulnerability alerts if specified + if vulnerabilityAlerts, ok := settings["vulnerability_alerts"].(bool); ok { + if repository.SecurityAndAnalysis.AdvancedSecurity == nil { + repository.SecurityAndAnalysis.AdvancedSecurity = &github.AdvancedSecurity{} + } + if vulnerabilityAlerts { + repository.SecurityAndAnalysis.AdvancedSecurity.Status = github.Ptr("enabled") + } else { + repository.SecurityAndAnalysis.AdvancedSecurity.Status = github.Ptr("disabled") + } + } + + // Update other security settings + settingsJSON, err := json.Marshal(settings) + if err != nil { + return nil, fmt.Errorf("failed to marshal settings: %w", err) + } + + var securitySettings github.SecurityAndAnalysis + if err := json.Unmarshal(settingsJSON, &securitySettings); err != nil { + return nil, fmt.Errorf("failed to unmarshal settings: %w", err) + } + + // Merge the new settings with existing ones + if securitySettings.AdvancedSecurity != nil { + repository.SecurityAndAnalysis.AdvancedSecurity = securitySettings.AdvancedSecurity + } + if securitySettings.SecretScanning != nil { + repository.SecurityAndAnalysis.SecretScanning = securitySettings.SecretScanning + } + if securitySettings.SecretScanningPushProtection != nil { + repository.SecurityAndAnalysis.SecretScanningPushProtection = securitySettings.SecretScanningPushProtection + } + + // Update the repository + updatedRepo, _, err := client.Repositories.Edit(ctx, owner, repo, &github.Repository{ + SecurityAndAnalysis: repository.SecurityAndAnalysis, + }) + if err != nil { + return nil, fmt.Errorf("failed to update repository settings: %w", err) + } + + // Return complete security settings + response, err := json.Marshal(updatedRepo.SecurityAndAnalysis) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(response)), nil + } +} + +// GetDependabotSecurityUpdatesStatus checks if Dependabot security updates are enabled +func GetDependabotSecurityUpdatesStatus(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("get_dependabot_security_updates_status", + mcp.WithDescription(t("TOOL_GET_DEPENDABOT_SECURITY_UPDATES_STATUS_DESCRIPTION", "Check if Dependabot security updates are enabled for a repository")), + mcp.WithString("owner", + mcp.Required(), + mcp.Description(t("PARAM_OWNER_DESCRIPTION", "Repository owner")), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description(t("PARAM_REPO_DESCRIPTION", "Repository name")), + ), + ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + owner, ok := request.Params.Arguments["owner"].(string) + if !ok { + return nil, fmt.Errorf("missing required parameter: owner") + } + + repo, ok := request.Params.Arguments["repo"].(string) + if !ok { + return nil, fmt.Errorf("missing required parameter: repo") + } + + status, _, err := client.Repositories.GetAutomatedSecurityFixes(ctx, owner, repo) + if err != nil { + return nil, fmt.Errorf("failed to get Dependabot security updates status: %w", err) + } + + response, err := json.Marshal(status) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(response)), nil + } +} + +// EnableDependabotSecurityUpdates and DisableDependabotSecurityUpdates are currently disabled. +// Issue: There is a discrepancy in GitHub's API behavior regarding Dependabot security updates: +// 1. Public repositories should have Dependabot alerts enabled by default +// 2. However, the API still requires explicit enabling of vulnerability alerts +// 3. This creates a confusing user experience where the system says one thing but behaves differently +// 4. The functionality needs to be investigated and fixed before being re-enabled +// See: https://github.com/github/github-mcp-server/issues/176 + +// EnableDependabotSecurityUpdates enables Dependabot security updates for a repository +// func EnableDependabotSecurityUpdates(client *github.Client, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { +// return mcp.NewTool("enable_dependabot_security_updates", +// mcp.WithDescription(t("TOOL_ENABLE_DEPENDABOT_SECURITY_UPDATES_DESCRIPTION", "Enable Dependabot security updates for a repository")), +// mcp.WithString("owner", +// mcp.Required(), +// mcp.Description(t("PARAM_OWNER_DESCRIPTION", "Repository owner")), +// ), +// mcp.WithString("repo", +// mcp.Required(), +// mcp.Description(t("PARAM_REPO_DESCRIPTION", "Repository name")), +// ), +// ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { +// return nil, fmt.Errorf("this functionality is currently disabled due to GitHub API behavior discrepancy") +// } +// } + +// DisableDependabotSecurityUpdates disables Dependabot security updates for a repository +// func DisableDependabotSecurityUpdates(client *github.Client, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { +// return mcp.NewTool("disable_dependabot_security_updates", +// mcp.WithDescription(t("TOOL_DISABLE_DEPENDABOT_SECURITY_UPDATES_DESCRIPTION", "Disable Dependabot security updates for a repository")), +// mcp.WithString("owner", +// mcp.Required(), +// mcp.Description(t("PARAM_OWNER_DESCRIPTION", "Repository owner")), +// ), +// mcp.WithString("repo", +// mcp.Required(), +// mcp.Description(t("PARAM_REPO_DESCRIPTION", "Repository name")), +// ), +// ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { +// return nil, fmt.Errorf("this functionality is currently disabled due to GitHub API behavior discrepancy") +// } +// } \ No newline at end of file diff --git a/pkg/github/security_test.go b/pkg/github/security_test.go new file mode 100644 index 00000000..f7d0beb8 --- /dev/null +++ b/pkg/github/security_test.go @@ -0,0 +1,470 @@ +package github + +import ( + "context" + "encoding/json" + "net/http" + "testing" + + "github.com/github/github-mcp-server/pkg/translations" + "github.com/google/go-github/v69/github" + "github.com/migueleliasweb/go-github-mock/src/mock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_GetSecuritySettings(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := GetSecuritySettings(mockClient, translations.NullTranslationHelper) + + assert.Equal(t, "get_security_settings", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "owner") + assert.Contains(t, tool.InputSchema.Properties, "repo") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) + + // Setup mock security settings + mockSettings := &github.SecurityAndAnalysis{ + AdvancedSecurity: &github.AdvancedSecurity{ + Status: github.Ptr("enabled"), + }, + SecretScanning: &github.SecretScanning{ + Status: github.Ptr("enabled"), + }, + SecretScanningPushProtection: &github.SecretScanningPushProtection{ + Status: github.Ptr("enabled"), + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedResult *github.SecurityAndAnalysis + expectedErrMsg string + }{ + { + name: "successful security settings retrieval", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetReposByOwnerByRepo, + &github.Repository{ + SecurityAndAnalysis: mockSettings, + }, + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + }, + expectError: false, + expectedResult: mockSettings, + }, + { + name: "repository not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposByOwnerByRepo, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Repository not found"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "nonexistent", + }, + expectError: true, + expectedErrMsg: "failed to get repository settings", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := GetSecuritySettings(client, translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + + require.NoError(t, err) + textContent := getTextResult(t, result) + + // Unmarshal and verify the result + var returnedSettings github.SecurityAndAnalysis + err = json.Unmarshal([]byte(textContent.Text), &returnedSettings) + require.NoError(t, err) + + assert.Equal(t, *tc.expectedResult.AdvancedSecurity.Status, *returnedSettings.AdvancedSecurity.Status) + assert.Equal(t, *tc.expectedResult.SecretScanning.Status, *returnedSettings.SecretScanning.Status) + assert.Equal(t, *tc.expectedResult.SecretScanningPushProtection.Status, *returnedSettings.SecretScanningPushProtection.Status) + }) + } +} + +func Test_GetDependabotSecurityUpdatesStatus(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := GetDependabotSecurityUpdatesStatus(mockClient, translations.NullTranslationHelper) + + assert.Equal(t, "get_dependabot_security_updates_status", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "owner") + assert.Contains(t, tool.InputSchema.Properties, "repo") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) + + // Setup mock response + mockResponse := &github.AutomatedSecurityFixes{ + Enabled: github.Ptr(true), + Paused: github.Ptr(false), + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedResult *github.AutomatedSecurityFixes + expectedErrMsg string + }{ + { + name: "successful status retrieval", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetReposAutomatedSecurityFixesByOwnerByRepo, + mockResponse, + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + }, + expectError: false, + expectedResult: mockResponse, + }, + { + name: "repository not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposAutomatedSecurityFixesByOwnerByRepo, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Repository not found"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "nonexistent", + }, + expectError: true, + expectedErrMsg: "failed to get Dependabot security updates status", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := GetDependabotSecurityUpdatesStatus(client, translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + + require.NoError(t, err) + textContent := getTextResult(t, result) + + // Unmarshal and verify the result + var returnedStatus github.AutomatedSecurityFixes + err = json.Unmarshal([]byte(textContent.Text), &returnedStatus) + require.NoError(t, err) + + assert.Equal(t, *tc.expectedResult.Enabled, *returnedStatus.Enabled) + assert.Equal(t, *tc.expectedResult.Paused, *returnedStatus.Paused) + }) + } +} + +func Test_UpdateSecuritySettings(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := UpdateSecuritySettings(mockClient, translations.NullTranslationHelper) + + assert.Equal(t, "update_security_settings", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "owner") + assert.Contains(t, tool.InputSchema.Properties, "repo") + assert.Contains(t, tool.InputSchema.Properties, "settings") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "settings"}) + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedResult *github.SecurityAndAnalysis + expectedErrMsg string + }{ + { + name: "successful update with vulnerability alerts", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetReposByOwnerByRepo, + &github.Repository{ + SecurityAndAnalysis: &github.SecurityAndAnalysis{ + AdvancedSecurity: &github.AdvancedSecurity{ + Status: github.Ptr("disabled"), + }, + SecretScanning: &github.SecretScanning{ + Status: github.Ptr("disabled"), + }, + }, + }, + ), + mock.WithRequestMatchHandler( + mock.PatchReposByOwnerByRepo, + expectRequestBody(t, map[string]interface{}{ + "security_and_analysis": map[string]interface{}{ + "advanced_security": map[string]interface{}{ + "status": "enabled", + }, + "secret_scanning": map[string]interface{}{ + "status": "disabled", + }, + }, + }).andThen( + mockResponse(t, http.StatusOK, &github.Repository{ + SecurityAndAnalysis: &github.SecurityAndAnalysis{ + AdvancedSecurity: &github.AdvancedSecurity{ + Status: github.Ptr("enabled"), + }, + SecretScanning: &github.SecretScanning{ + Status: github.Ptr("disabled"), + }, + }, + }), + ), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "settings": map[string]interface{}{ + "vulnerability_alerts": true, + }, + }, + expectError: false, + expectedResult: &github.SecurityAndAnalysis{ + AdvancedSecurity: &github.AdvancedSecurity{ + Status: github.Ptr("enabled"), + }, + SecretScanning: &github.SecretScanning{ + Status: github.Ptr("disabled"), + }, + }, + }, + { + name: "successful update with multiple settings", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetReposByOwnerByRepo, + &github.Repository{ + SecurityAndAnalysis: &github.SecurityAndAnalysis{}, + }, + ), + mock.WithRequestMatchHandler( + mock.PatchReposByOwnerByRepo, + expectRequestBody(t, map[string]interface{}{ + "security_and_analysis": map[string]interface{}{ + "advanced_security": map[string]interface{}{ + "status": "enabled", + }, + "secret_scanning": map[string]interface{}{ + "status": "enabled", + }, + "secret_scanning_push_protection": map[string]interface{}{ + "status": "enabled", + }, + }, + }).andThen( + mockResponse(t, http.StatusOK, &github.Repository{ + SecurityAndAnalysis: &github.SecurityAndAnalysis{ + AdvancedSecurity: &github.AdvancedSecurity{ + Status: github.Ptr("enabled"), + }, + SecretScanning: &github.SecretScanning{ + Status: github.Ptr("enabled"), + }, + SecretScanningPushProtection: &github.SecretScanningPushProtection{ + Status: github.Ptr("enabled"), + }, + }, + }), + ), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "settings": map[string]interface{}{ + "advanced_security": map[string]interface{}{ + "status": "enabled", + }, + "secret_scanning": map[string]interface{}{ + "status": "enabled", + }, + "secret_scanning_push_protection": map[string]interface{}{ + "status": "enabled", + }, + }, + }, + expectError: false, + expectedResult: &github.SecurityAndAnalysis{ + AdvancedSecurity: &github.AdvancedSecurity{ + Status: github.Ptr("enabled"), + }, + SecretScanning: &github.SecretScanning{ + Status: github.Ptr("enabled"), + }, + SecretScanningPushProtection: &github.SecretScanningPushProtection{ + Status: github.Ptr("enabled"), + }, + }, + }, + { + name: "repository not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposByOwnerByRepo, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Repository not found"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "nonexistent", + "settings": map[string]interface{}{ + "vulnerability_alerts": true, + }, + }, + expectError: true, + expectedErrMsg: "failed to get repository settings", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := UpdateSecuritySettings(client, translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + + require.NoError(t, err) + textContent := getTextResult(t, result) + + // Unmarshal and verify the result + var returnedSettings github.SecurityAndAnalysis + err = json.Unmarshal([]byte(textContent.Text), &returnedSettings) + require.NoError(t, err) + + if tc.expectedResult.AdvancedSecurity != nil { + assert.Equal(t, *tc.expectedResult.AdvancedSecurity.Status, *returnedSettings.AdvancedSecurity.Status) + } + if tc.expectedResult.SecretScanning != nil { + assert.Equal(t, *tc.expectedResult.SecretScanning.Status, *returnedSettings.SecretScanning.Status) + } + if tc.expectedResult.SecretScanningPushProtection != nil { + assert.Equal(t, *tc.expectedResult.SecretScanningPushProtection.Status, *returnedSettings.SecretScanningPushProtection.Status) + } + }) + } +} + +// Test_EnableDependabotSecurityUpdates and Test_DisableDependabotSecurityUpdates are currently disabled. +// See the comment in security.go for details about the GitHub API behavior discrepancy. +// func Test_EnableDependabotSecurityUpdates(t *testing.T) { +// // Verify tool definition +// mockClient := github.NewClient(nil) +// tool, _ := EnableDependabotSecurityUpdates(mockClient, translations.NullTranslationHelper) +// +// assert.Equal(t, "enable_dependabot_security_updates", tool.Name) +// assert.NotEmpty(t, tool.Description) +// assert.Contains(t, tool.InputSchema.Properties, "owner") +// assert.Contains(t, tool.InputSchema.Properties, "repo") +// assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) +// +// // Test that the function returns an error indicating the functionality is disabled +// _, handler := EnableDependabotSecurityUpdates(mockClient, translations.NullTranslationHelper) +// request := createMCPRequest(map[string]interface{}{ +// "owner": "owner", +// "repo": "repo", +// }) +// +// result, err := handler(context.Background(), request) +// assert.Error(t, err) +// assert.Contains(t, err.Error(), "this functionality is currently disabled") +// assert.Nil(t, result) +// } +// +// // Test_DisableDependabotSecurityUpdates verifies that the disabled functionality returns an appropriate error +// func Test_DisableDependabotSecurityUpdates(t *testing.T) { +// // Verify tool definition +// mockClient := github.NewClient(nil) +// tool, _ := DisableDependabotSecurityUpdates(mockClient, translations.NullTranslationHelper) +// +// assert.Equal(t, "disable_dependabot_security_updates", tool.Name) +// assert.NotEmpty(t, tool.Description) +// assert.Contains(t, tool.InputSchema.Properties, "owner") +// assert.Contains(t, tool.InputSchema.Properties, "repo") +// assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) +// +// // Test that the function returns an error indicating the functionality is disabled +// _, handler := DisableDependabotSecurityUpdates(mockClient, translations.NullTranslationHelper) +// request := createMCPRequest(map[string]interface{}{ +// "owner": "owner", +// "repo": "repo", +// }) +// +// result, err := handler(context.Background(), request) +// assert.Error(t, err) +// assert.Contains(t, err.Error(), "this functionality is currently disabled") +// assert.Nil(t, result) +// } diff --git a/pkg/github/server.go b/pkg/github/server.go index 5852d581..25fa5b95 100644 --- a/pkg/github/server.go +++ b/pkg/github/server.go @@ -77,6 +77,18 @@ func NewServer(client *github.Client, version string, readOnly bool, t translati // Add GitHub tools - Code Scanning s.AddTool(GetCodeScanningAlert(client, t)) s.AddTool(ListCodeScanningAlerts(client, t)) + + // Add GitHub tools - Security + s.AddTool(GetSecuritySettings(client, t)) + s.AddTool(GetDependabotSecurityUpdatesStatus(client, t)) + if !readOnly { + s.AddTool(UpdateSecuritySettings(client, t)) + // Dependabot security update tools are currently disabled due to GitHub API behavior discrepancy + // See: https://github.com/github/github-mcp-server/issues/176 + // s.AddTool(EnableDependabotSecurityUpdates(client, t)) + // s.AddTool(DisableDependabotSecurityUpdates(client, t)) + } + return s } From 29ce2400a14146dab91834c7fbb38e14c672ff72 Mon Sep 17 00:00:00 2001 From: smadi0x86 <75253629+smadi0x86@users.noreply.github.com> Date: Wed, 9 Apr 2025 08:04:59 +0300 Subject: [PATCH 2/2] Update security.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- pkg/github/security.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/github/security.go b/pkg/github/security.go index 0446be3e..a0ac43fc 100644 --- a/pkg/github/security.go +++ b/pkg/github/security.go @@ -129,7 +129,9 @@ func UpdateSecuritySettings(client *github.Client, t translations.TranslationHel // Merge the new settings with existing ones if securitySettings.AdvancedSecurity != nil { - repository.SecurityAndAnalysis.AdvancedSecurity = securitySettings.AdvancedSecurity + if repository.SecurityAndAnalysis.AdvancedSecurity == nil || repository.SecurityAndAnalysis.AdvancedSecurity.Status == "" { + repository.SecurityAndAnalysis.AdvancedSecurity = securitySettings.AdvancedSecurity + } } if securitySettings.SecretScanning != nil { repository.SecurityAndAnalysis.SecretScanning = securitySettings.SecretScanning