From a572578e8c4ca71e56100538013d75cddd792884 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 4 Feb 2026 20:35:37 +0000 Subject: [PATCH] fix(security): remediate SonarQube vulnerabilities (S1192, S3776) - S1192: Extract duplicated 'Sort order' string to sortOrderDescription constant - S3776 search.go: Reduce cognitive complexity of userOrOrgHandler by extracting: - userSearchParams struct for parameter grouping - extractUserSearchParams for parameter extraction - convertToMinimalUser for user conversion - buildMinimalSearchResult for result building - S3776 repository_resource.go: Reduce cognitive complexity of RepositoryResourceContentsHandler by extracting: - resourceRequestParams struct for parameter grouping - extractRequiredStringArg/extractOptionalStringArg for argument extraction - extractResourceParams for parameter extraction - setRefOptions for ref option setting - handlePRRef for PR reference handling - determineMimeType for MIME type detection - buildResourceContents for content building - handleRawContentResponse for response handling Co-Authored-By: parker.duff@codeium.com --- pkg/github/repository_resource.go | 251 ++++++++++++++++++------------ pkg/github/search.go | 132 ++++++++++------ 2 files changed, 229 insertions(+), 154 deletions(-) diff --git a/pkg/github/repository_resource.go b/pkg/github/repository_resource.go index a454db630..83f5d603a 100644 --- a/pkg/github/repository_resource.go +++ b/pkg/github/repository_resource.go @@ -64,129 +64,174 @@ func GetRepositoryResourcePrContent(getClient GetClientFn, getRawClient raw.GetR RepositoryResourceContentsHandler(getClient, getRawClient) } +// resourceRequestParams holds the extracted parameters from a resource request +type resourceRequestParams struct { + owner string + repo string + path string +} + +// extractRequiredStringArg extracts a required string argument from the request +func extractRequiredStringArg(args map[string]interface{}, key string) (string, error) { + val, ok := args[key].([]string) + if !ok || len(val) == 0 { + return "", fmt.Errorf("%s is required", key) + } + return val[0], nil +} + +// extractOptionalStringArg extracts an optional string argument from the request +func extractOptionalStringArg(args map[string]interface{}, key string) (string, bool) { + val, ok := args[key].([]string) + if !ok || len(val) == 0 { + return "", false + } + return val[0], true +} + +// extractResourceParams extracts owner, repo, and path from the request arguments +func extractResourceParams(args map[string]interface{}) (*resourceRequestParams, error) { + owner, err := extractRequiredStringArg(args, "owner") + if err != nil { + return nil, err + } + repo, err := extractRequiredStringArg(args, "repo") + if err != nil { + return nil, err + } + path := "" + if p, ok := args["path"].([]string); ok { + path = strings.Join(p, "/") + } + return &resourceRequestParams{owner: owner, repo: repo, path: path}, nil +} + +// setRefOptions sets the ref options based on the request arguments +func setRefOptions(args map[string]interface{}, opts *github.RepositoryContentGetOptions, rawOpts *raw.ContentOpts) { + if sha, ok := extractOptionalStringArg(args, "sha"); ok { + opts.Ref = sha + rawOpts.SHA = sha + } + if branch, ok := extractOptionalStringArg(args, "branch"); ok { + opts.Ref = "refs/heads/" + branch + rawOpts.Ref = "refs/heads/" + branch + } + if tag, ok := extractOptionalStringArg(args, "tag"); ok { + opts.Ref = "refs/tags/" + tag + rawOpts.Ref = "refs/tags/" + tag + } +} + +// handlePRRef fetches the PR and sets the SHA for the ref options +func handlePRRef(ctx context.Context, args map[string]interface{}, owner, repo string, getClient GetClientFn, opts *github.RepositoryContentGetOptions, rawOpts *raw.ContentOpts) error { + prNumberStr, ok := extractOptionalStringArg(args, "prNumber") + if !ok { + return nil + } + githubClient, err := getClient(ctx) + if err != nil { + return fmt.Errorf("failed to get GitHub client: %w", err) + } + prNum, err := strconv.Atoi(prNumberStr) + if err != nil { + return fmt.Errorf("invalid pull request number: %w", err) + } + pr, _, err := githubClient.PullRequests.Get(ctx, owner, repo, prNum) + if err != nil { + return fmt.Errorf("failed to get pull request: %w", err) + } + sha := pr.GetHead().GetSHA() + rawOpts.SHA = sha + opts.Ref = sha + return nil +} + +// determineMimeType determines the MIME type for a file based on extension and response headers +func determineMimeType(path string, contentType string) string { + ext := filepath.Ext(path) + if ext == ".md" { + return "text/markdown" + } + if contentType != "" { + return contentType + } + return mime.TypeByExtension(ext) +} + +// buildResourceContents builds the appropriate resource contents based on MIME type +func buildResourceContents(uri string, mimeType string, content []byte) []mcp.ResourceContents { + if strings.HasPrefix(mimeType, "text") || strings.HasPrefix(mimeType, "application") { + return []mcp.ResourceContents{ + mcp.TextResourceContents{ + URI: uri, + MIMEType: mimeType, + Text: string(content), + }, + } + } + return []mcp.ResourceContents{ + mcp.BlobResourceContents{ + URI: uri, + MIMEType: mimeType, + Blob: base64.StdEncoding.EncodeToString(content), + }, + } +} + +// handleRawContentResponse processes the raw content response and returns resource contents +func handleRawContentResponse(resp *http.Response, uri string, path string) ([]mcp.ResourceContents, error) { + if resp.StatusCode == http.StatusOK { + mimeType := determineMimeType(path, resp.Header.Get("Content-Type")) + content, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read file content: %w", err) + } + return buildResourceContents(uri, mimeType, content), nil + } + if resp.StatusCode != http.StatusNotFound { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return nil, fmt.Errorf("failed to fetch raw content: %s", string(body)) + } + return nil, errors.New("404 Not Found") +} + // RepositoryResourceContentsHandler returns a handler function for repository content requests. func RepositoryResourceContentsHandler(getClient GetClientFn, getRawClient raw.GetRawClientFn) func(ctx context.Context, request mcp.ReadResourceRequest) ([]mcp.ResourceContents, error) { return func(ctx context.Context, request mcp.ReadResourceRequest) ([]mcp.ResourceContents, error) { - // the matcher will give []string with one element - // https://github.com/mark3labs/mcp-go/pull/54 - o, ok := request.Params.Arguments["owner"].([]string) - if !ok || len(o) == 0 { - return nil, errors.New("owner is required") - } - owner := o[0] - - r, ok := request.Params.Arguments["repo"].([]string) - if !ok || len(r) == 0 { - return nil, errors.New("repo is required") + params, err := extractResourceParams(request.Params.Arguments) + if err != nil { + return nil, err } - repo := r[0] - // path should be a joined list of the path parts - path := "" - p, ok := request.Params.Arguments["path"].([]string) - if ok { - path = strings.Join(p, "/") + if params.path == "" || strings.HasSuffix(params.path, "/") { + return nil, fmt.Errorf("directories are not supported: %s", params.path) } opts := &github.RepositoryContentGetOptions{} rawOpts := &raw.ContentOpts{} - sha, ok := request.Params.Arguments["sha"].([]string) - if ok && len(sha) > 0 { - opts.Ref = sha[0] - rawOpts.SHA = sha[0] - } + setRefOptions(request.Params.Arguments, opts, rawOpts) - branch, ok := request.Params.Arguments["branch"].([]string) - if ok && len(branch) > 0 { - opts.Ref = "refs/heads/" + branch[0] - rawOpts.Ref = "refs/heads/" + branch[0] + if err := handlePRRef(ctx, request.Params.Arguments, params.owner, params.repo, getClient, opts, rawOpts); err != nil { + return nil, err } - tag, ok := request.Params.Arguments["tag"].([]string) - if ok && len(tag) > 0 { - opts.Ref = "refs/tags/" + tag[0] - rawOpts.Ref = "refs/tags/" + tag[0] - } - prNumber, ok := request.Params.Arguments["prNumber"].([]string) - if ok && len(prNumber) > 0 { - // fetch the PR from the API to get the latest commit and use SHA - githubClient, err := getClient(ctx) - if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) - } - prNum, err := strconv.Atoi(prNumber[0]) - if err != nil { - return nil, fmt.Errorf("invalid pull request number: %w", err) - } - pr, _, err := githubClient.PullRequests.Get(ctx, owner, repo, prNum) - if err != nil { - return nil, fmt.Errorf("failed to get pull request: %w", err) - } - sha := pr.GetHead().GetSHA() - rawOpts.SHA = sha - opts.Ref = sha - } - // if it's a directory - if path == "" || strings.HasSuffix(path, "/") { - return nil, fmt.Errorf("directories are not supported: %s", path) - } rawClient, err := getRawClient(ctx) - if err != nil { return nil, fmt.Errorf("failed to get GitHub raw content client: %w", err) } - resp, err := rawClient.GetRawContent(ctx, owner, repo, path, rawOpts) + resp, err := rawClient.GetRawContent(ctx, params.owner, params.repo, params.path, rawOpts) + if err != nil { + return nil, fmt.Errorf("failed to get raw content: %w", err) + } defer func() { _ = resp.Body.Close() }() - // If the raw content is not found, we will fall back to the GitHub API (in case it is a directory) - switch { - case err != nil: - return nil, fmt.Errorf("failed to get raw content: %w", err) - case resp.StatusCode == http.StatusOK: - ext := filepath.Ext(path) - mimeType := resp.Header.Get("Content-Type") - if ext == ".md" { - mimeType = "text/markdown" - } else if mimeType == "" { - mimeType = mime.TypeByExtension(ext) - } - - content, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read file content: %w", err) - } - - switch { - case strings.HasPrefix(mimeType, "text"), strings.HasPrefix(mimeType, "application"): - return []mcp.ResourceContents{ - mcp.TextResourceContents{ - URI: request.Params.URI, - MIMEType: mimeType, - Text: string(content), - }, - }, nil - default: - return []mcp.ResourceContents{ - mcp.BlobResourceContents{ - URI: request.Params.URI, - MIMEType: mimeType, - Blob: base64.StdEncoding.EncodeToString(content), - }, - }, nil - } - case resp.StatusCode != http.StatusNotFound: - // If we got a response but it is not 200 OK, we return an error - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) - } - return nil, fmt.Errorf("failed to fetch raw content: %s", string(body)) - default: - // This should be unreachable because GetContents should return an error if neither file nor directory content is found. - return nil, errors.New("404 Not Found") - } + + return handleRawContentResponse(resp, request.Params.URI, params.path) } } diff --git a/pkg/github/search.go b/pkg/github/search.go index 5106b84d8..bc39cd45f 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -13,6 +13,9 @@ import ( "github.com/mark3labs/mcp-go/server" ) +// sortOrderDescription is a constant for the duplicated "Sort order" description string (S1192) +const sortOrderDescription = "Sort order" + // SearchRepositories creates a tool to search for GitHub repositories. func SearchRepositories(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("search_repositories", @@ -91,7 +94,7 @@ func SearchCode(getClient GetClientFn, t translations.TranslationHelperFunc) (to mcp.Description("Sort field ('indexed' only)"), ), mcp.WithString("order", - mcp.Description("Sort order"), + mcp.Description(sortOrderDescription), mcp.Enum("asc", "desc"), ), WithPagination(), @@ -168,31 +171,86 @@ type MinimalSearchUsersResult struct { Items []MinimalUser `json:"items"` } +// userSearchParams holds the extracted parameters for user/org search +type userSearchParams struct { + query string + sort string + order string + pagination PaginationParams +} + +// extractUserSearchParams extracts and validates search parameters from the request +func extractUserSearchParams(request mcp.CallToolRequest) (*userSearchParams, error) { + query, err := RequiredParam[string](request, "query") + if err != nil { + return nil, err + } + sort, err := OptionalParam[string](request, "sort") + if err != nil { + return nil, err + } + order, err := OptionalParam[string](request, "order") + if err != nil { + return nil, err + } + pag, err := OptionalPaginationParams(request) + if err != nil { + return nil, err + } + return &userSearchParams{ + query: query, + sort: sort, + order: order, + pagination: pag, + }, nil +} + +// convertToMinimalUser converts a GitHub User to MinimalUser +func convertToMinimalUser(user *github.User) *MinimalUser { + if user.Login == nil { + return nil + } + mu := &MinimalUser{Login: *user.Login} + if user.ID != nil { + mu.ID = *user.ID + } + if user.HTMLURL != nil { + mu.ProfileURL = *user.HTMLURL + } + if user.AvatarURL != nil { + mu.AvatarURL = *user.AvatarURL + } + return mu +} + +// buildMinimalSearchResult converts GitHub search results to MinimalSearchUsersResult +func buildMinimalSearchResult(result *github.UsersSearchResult) *MinimalSearchUsersResult { + minimalUsers := make([]MinimalUser, 0, len(result.Users)) + for _, user := range result.Users { + if mu := convertToMinimalUser(user); mu != nil { + minimalUsers = append(minimalUsers, *mu) + } + } + return &MinimalSearchUsersResult{ + TotalCount: result.GetTotal(), + IncompleteResults: result.GetIncompleteResults(), + Items: minimalUsers, + } +} + func userOrOrgHandler(accountType string, getClient GetClientFn) server.ToolHandlerFunc { return func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - query, err := RequiredParam[string](request, "query") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - sort, err := OptionalParam[string](request, "sort") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - order, err := OptionalParam[string](request, "order") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - pagination, err := OptionalPaginationParams(request) + params, err := extractUserSearchParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } opts := &github.SearchOptions{ - Sort: sort, - Order: order, + Sort: params.sort, + Order: params.order, ListOptions: github.ListOptions{ - PerPage: pagination.perPage, - Page: pagination.page, + PerPage: params.pagination.perPage, + Page: params.pagination.page, }, } @@ -201,11 +259,11 @@ func userOrOrgHandler(accountType string, getClient GetClientFn) server.ToolHand return nil, fmt.Errorf("failed to get GitHub client: %w", err) } - searchQuery := "type:" + accountType + " " + query + searchQuery := "type:" + accountType + " " + params.query result, resp, err := client.Search.Users(ctx, searchQuery, opts) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, - fmt.Sprintf("failed to search %ss with query '%s'", accountType, query), + fmt.Sprintf("failed to search %ss with query '%s'", accountType, params.query), resp, err, ), nil @@ -220,35 +278,7 @@ func userOrOrgHandler(accountType string, getClient GetClientFn) server.ToolHand return mcp.NewToolResultError(fmt.Sprintf("failed to search %ss: %s", accountType, string(body))), nil } - minimalUsers := make([]MinimalUser, 0, len(result.Users)) - - for _, user := range result.Users { - if user.Login != nil { - mu := MinimalUser{Login: *user.Login} - if user.ID != nil { - mu.ID = *user.ID - } - if user.HTMLURL != nil { - mu.ProfileURL = *user.HTMLURL - } - if user.AvatarURL != nil { - mu.AvatarURL = *user.AvatarURL - } - minimalUsers = append(minimalUsers, mu) - } - } - minimalResp := &MinimalSearchUsersResult{ - TotalCount: result.GetTotal(), - IncompleteResults: result.GetIncompleteResults(), - Items: minimalUsers, - } - if result.Total != nil { - minimalResp.TotalCount = *result.Total - } - if result.IncompleteResults != nil { - minimalResp.IncompleteResults = *result.IncompleteResults - } - + minimalResp := buildMinimalSearchResult(result) r, err := json.Marshal(minimalResp) if err != nil { return nil, fmt.Errorf("failed to marshal response: %w", err) @@ -274,7 +304,7 @@ func SearchUsers(getClient GetClientFn, t translations.TranslationHelperFunc) (t mcp.Enum("followers", "repositories", "joined"), ), mcp.WithString("order", - mcp.Description("Sort order"), + mcp.Description(sortOrderDescription), mcp.Enum("asc", "desc"), ), WithPagination(), @@ -298,7 +328,7 @@ func SearchOrgs(getClient GetClientFn, t translations.TranslationHelperFunc) (to mcp.Enum("followers", "repositories", "joined"), ), mcp.WithString("order", - mcp.Description("Sort order"), + mcp.Description(sortOrderDescription), mcp.Enum("asc", "desc"), ), WithPagination(),