lint fixes

Author: tstromberg

batch_processor.go

@@ -255,34 +255,73 @@ func (rf *ReviewerFinder) prsForOrgWithBatchSize(ctx context.Context, org string
 
 // parseOrgPRsFromGraphQL parses PRs from GraphQL response.
 func (rf *ReviewerFinder) parseOrgPRsFromGraphQL(result map[string]any) (prs []*PullRequest, hasNextPage bool, cursor string) {
+	repos := rf.extractOrgRepositories(result)
+	if repos == nil {
+		return prs, false, ""
+	}
 
-	// Navigate through the GraphQL response structure
-	if data, ok := result["data"].(map[string]any); ok {
-		if org, ok := data["organization"].(map[string]any); ok {
-			if repos, ok := org["repositories"].(map[string]any); ok {
-				// Get pagination info
-				if pageInfo, ok := repos["pageInfo"].(map[string]any); ok {
-					if next, ok := pageInfo["hasNextPage"].(bool); ok {
-						hasNextPage = next
-					}
-					if endCursor, ok := pageInfo["endCursor"].(string); ok {
-						cursor = endCursor
-					}
-				}
+	hasNextPage, cursor = rf.extractPaginationInfo(repos)
+	prs = rf.extractPRsFromRepos(repos)
 
-				// Process repositories
-				if nodes, ok := repos["nodes"].([]any); ok {
-					for _, node := range nodes {
-						if repo, ok := node.(map[string]any); ok {
-							prs = append(prs, rf.parsePRsFromRepo(repo)...)
-						}
-					}
-				}
-			}
+	return prs, hasNextPage, cursor
+}
+
+// extractOrgRepositories extracts the repositories object from org GraphQL response.
+func (*ReviewerFinder) extractOrgRepositories(result map[string]any) map[string]any {
+	data, ok := result["data"].(map[string]any)
+	if !ok {
+		return nil
+	}
+
+	org, ok := data["organization"].(map[string]any)
+	if !ok {
+		return nil
+	}
+
+	repos, ok := org["repositories"].(map[string]any)
+	if !ok {
+		return nil
+	}
+
+	return repos
+}
+
+// extractPaginationInfo extracts pagination info from repositories response.
+func (*ReviewerFinder) extractPaginationInfo(repos map[string]any) (hasNextPage bool, cursor string) {
+	pageInfo, ok := repos["pageInfo"].(map[string]any)
+	if !ok {
+		return false, ""
+	}
+
+	if next, ok := pageInfo["hasNextPage"].(bool); ok {
+		hasNextPage = next
+	}
+
+	if endCursor, ok := pageInfo["endCursor"].(string); ok {
+		cursor = endCursor
+	}
+
+	return hasNextPage, cursor
+}
+
+// extractPRsFromRepos extracts PRs from repository nodes.
+func (rf *ReviewerFinder) extractPRsFromRepos(repos map[string]any) []*PullRequest {
+	var prs []*PullRequest
+
+	nodes, ok := repos["nodes"].([]any)
+	if !ok {
+		return prs
+	}
+
+	for _, node := range nodes {
+		repo, ok := node.(map[string]any)
+		if !ok {
+			continue
 		}
+		prs = append(prs, rf.parsePRsFromRepo(repo)...)
 	}
 
-	return prs, hasNextPage, cursor
+	return prs
 }
 
 // parsePRsFromRepo parses PRs from a repository node.
@@ -323,26 +362,43 @@ func (rf *ReviewerFinder) parsePRFromGraphQL(prData map[string]any, owner, repo
 		Repository: repo,
 	}
 
-	// Parse basic fields
+	rf.parseBasicPRFields(prData, pr)
+	rf.parsePRAuthor(prData, pr)
+	rf.parsePRDates(prData, pr)
+	rf.parsePRAssignees(prData, pr)
+	rf.parsePRReviewers(prData, pr)
+	rf.parsePRLastCommit(prData, pr)
+
+	return pr
+}
+
+// parseBasicPRFields parses basic PR fields like number, title, and draft status.
+func (*ReviewerFinder) parseBasicPRFields(prData map[string]any, pr *PullRequest) {
 	if number, ok := prData["number"].(float64); ok {
 		pr.Number = int(number)
 	}
 	if title, ok := prData["title"].(string); ok {
 		pr.Title = title
 	}
-	// URL field doesn't exist in PullRequest struct, skip it
 	if isDraft, ok := prData["isDraft"].(bool); ok {
 		pr.Draft = isDraft
 	}
+}
 
-	// Parse author
-	if author, ok := prData["author"].(map[string]any); ok {
-		if login, ok := author["login"].(string); ok {
-			pr.Author = login
-		}
+// parsePRAuthor parses the PR author information.
+func (*ReviewerFinder) parsePRAuthor(prData map[string]any, pr *PullRequest) {
+	author, ok := prData["author"].(map[string]any)
+	if !ok {
+		return
 	}
 
-	// Parse dates
+	if login, ok := author["login"].(string); ok {
+		pr.Author = login
+	}
+}
+
+// parsePRDates parses PR creation and update dates.
+func (*ReviewerFinder) parsePRDates(prData map[string]any, pr *PullRequest) {
 	if createdAt, ok := prData["createdAt"].(string); ok {
 		if t, err := time.Parse(time.RFC3339, createdAt); err == nil {
 			pr.CreatedAt = t
@@ -353,60 +409,86 @@ func (rf *ReviewerFinder) parsePRFromGraphQL(prData map[string]any, owner, repo
 			pr.UpdatedAt = t
 		}
 	}
+}
 
-	// Parse assignees
-	if assignees, ok := prData["assignees"].(map[string]any); ok {
-		if nodes, ok := assignees["nodes"].([]any); ok {
-			for _, node := range nodes {
-				if assignee, ok := node.(map[string]any); ok {
-					if login, ok := assignee["login"].(string); ok {
-						pr.Assignees = append(pr.Assignees, login)
-					}
-				}
-			}
-		}
+// parsePRAssignees parses the list of PR assignees.
+func (*ReviewerFinder) parsePRAssignees(prData map[string]any, pr *PullRequest) {
+	assignees, ok := prData["assignees"].(map[string]any)
+	if !ok {
+		return
 	}
 
-	// Parse existing reviewers
-	if reviewRequests, ok := prData["reviewRequests"].(map[string]any); ok {
-		if nodes, ok := reviewRequests["nodes"].([]any); ok {
-			for _, node := range nodes {
-				if request, ok := node.(map[string]any); ok {
-					if reviewer, ok := request["requestedReviewer"].(map[string]any); ok {
-						if login, ok := reviewer["login"].(string); ok {
-							pr.Reviewers = append(pr.Reviewers, login)
-						}
-					}
-				}
-			}
+	nodes, ok := assignees["nodes"].([]any)
+	if !ok {
+		return
+	}
+
+	for _, node := range nodes {
+		assignee, ok := node.(map[string]any)
+		if !ok {
+			continue
+		}
+
+		if login, ok := assignee["login"].(string); ok {
+			pr.Assignees = append(pr.Assignees, login)
 		}
 	}
+}
 
-	// Parse last commit date
-	if commits, ok := prData["commits"].(map[string]any); ok {
-		if nodes, ok := commits["nodes"].([]any); ok && len(nodes) > 0 {
-			if commit, ok := nodes[0].(map[string]any); ok {
-				if commitData, ok := commit["commit"].(map[string]any); ok {
-					if committedDate, ok := commitData["committedDate"].(string); ok {
-						if t, err := time.Parse(time.RFC3339, committedDate); err == nil {
-							pr.LastCommit = t
-						}
-					}
-				}
-			}
+// parsePRReviewers parses the list of requested reviewers.
+func (*ReviewerFinder) parsePRReviewers(prData map[string]any, pr *PullRequest) {
+	reviewRequests, ok := prData["reviewRequests"].(map[string]any)
+	if !ok {
+		return
+	}
+
+	nodes, ok := reviewRequests["nodes"].([]any)
+	if !ok {
+		return
+	}
+
+	for _, node := range nodes {
+		request, ok := node.(map[string]any)
+		if !ok {
+			continue
+		}
+
+		reviewer, ok := request["requestedReviewer"].(map[string]any)
+		if !ok {
+			continue
+		}
+
+		if login, ok := reviewer["login"].(string); ok {
+			pr.Reviewers = append(pr.Reviewers, login)
 		}
 	}
+}
 
-	// LastReview will be fetched on-demand when needed for detailed analysis
-	// Removed from initial query to reduce GraphQL payload size
+// parsePRLastCommit parses the last commit date from the PR.
+func (*ReviewerFinder) parsePRLastCommit(prData map[string]any, pr *PullRequest) {
+	commits, ok := prData["commits"].(map[string]any)
+	if !ok {
+		return
+	}
+
+	nodes, ok := commits["nodes"].([]any)
+	if !ok || len(nodes) == 0 {
+		return
+	}
 
-	// Parse file statistics - these fields don't exist in our PullRequest struct
-	// We would need to fetch file details separately if needed
-	// additions, deletions, and changedFiles are available in the GraphQL response
-	// but our PullRequest struct doesn't have these fields directly
+	commit, ok := nodes[0].(map[string]any)
+	if !ok {
+		return
+	}
 
-	// Note: ChangedFiles array would need a separate query for file details
-	// For now, we'll fetch those on-demand when needed
+	commitData, ok := commit["commit"].(map[string]any)
+	if !ok {
+		return
+	}
 
-	return pr
+	if committedDate, ok := commitData["committedDate"].(string); ok {
+		if t, err := time.Parse(time.RFC3339, committedDate); err == nil {
+			pr.LastCommit = t
+		}
+	}
 }

better-reviewers

@@ -36,6 +36,10 @@ const (
 	prCountCacheTTL          = 6 * time.Hour       // PR count for workload balancing (default).
 	prCountFailureCacheTTL   = 10 * time.Minute    // Cache failures to avoid repeated API calls.
 	prStaleDaysThreshold     = 90                  // PRs older than this are considered stale.
+	maxTokenLength           = 100                 // Maximum expected length for GitHub tokens.
+	maxURLLength             = 500                 // Maximum URL length to validate.
+	maxPRNumber              = 999999              // Maximum PR number to validate.
+	maxGitHubNameLength      = 100                 // Maximum length for GitHub owner/repo names.
 
 	// API and pagination limits.
 	perPageLimit = 100 // GitHub API per_page limit
@@ -94,12 +98,12 @@ const (
 	reviewerWeightMultiplier = 0.5  // Weight multiplier for reviewers vs authors
 
 	// Overlap scoring constants.
-	contextMatchWeight = 0.7 // Weight for context matches in overlap scoring
+	contextMatchWeight  = 0.7 // Weight for context matches in overlap scoring
 	minOverlapThreshold = 5.0 // Minimum overlap score threshold
 
 	// Analysis limits.
-	maxRecentCommits       = 10 // Maximum recent commits to analyze
-	maxDirectoryReviewers  = 5  // Maximum directory reviewers to return
+	maxRecentCommits      = 10 // Maximum recent commits to analyze
+	maxDirectoryReviewers = 5  // Maximum directory reviewers to return
 
 	// Batch processing sizes.
 	defaultBatchSize = 20 // Default batch size for processing

constants.go

@@ -84,14 +84,23 @@ func newGitHubClient(ctx context.Context, useAppAuth bool) (*GitHubClient, error
 		isAppAuth = true
 		log.Print("[AUTH] Using GitHub App authentication")
 	} else {
-		// Use gh CLI authentication
+		// Use gh CLI authentication with validation
 		cmd := exec.CommandContext(ctx, "gh", "auth", "token")
 		output, err := cmd.Output()
 		if err != nil {
 			return nil, fmt.Errorf("failed to get GitHub token: %w", err)
 		}
 
 		token = strings.TrimSpace(string(output))
+		// Validate token format (should be alphanumeric with underscores)
+		if token == "" || len(token) > maxTokenLength {
+			return nil, errors.New("invalid token length from gh command")
+		}
+		for _, r := range token {
+			if (r < 'a' || r > 'z') && (r < 'A' || r > 'Z') && (r < '0' || r > '9') && r != '_' {
+				return nil, errors.New("invalid token format from gh command")
+			}
+		}
 		if token == "" {
 			return nil, errors.New("no GitHub token found")
 		}
@@ -116,7 +125,12 @@ func newGitHubClient(ctx context.Context, useAppAuth bool) (*GitHubClient, error
 
 // makeRequest makes an HTTP request to the GitHub API with retry logic.
 func (c *GitHubClient) makeRequest(ctx context.Context, method, apiURL string, body any) (*http.Response, error) {
-	log.Printf("[HTTP] %s %s", method, apiURL)
+	// Sanitize URL for logging - remove potential sensitive query parameters
+	sanitizedURL := apiURL
+	if idx := strings.Index(apiURL, "?"); idx != -1 {
+		sanitizedURL = apiURL[:idx] + "?[REDACTED]"
+	}
+	log.Printf("[HTTP] %s %s", method, sanitizedURL)
 
 	var resp *http.Response
 	err := retryWithBackoff(ctx, fmt.Sprintf("%s %s", method, apiURL), func() error {
@@ -175,7 +189,13 @@ func (c *GitHubClient) makeRequest(ctx context.Context, method, apiURL string, b
 		return nil, err
 	}
 
-	log.Printf("[HTTP] %s %s - Status: %d", method, apiURL, resp.StatusCode)
+	// Sanitize URL for logging (reuse variable)
+	if idx := strings.Index(apiURL, "?"); idx != -1 {
+		sanitizedURL = apiURL[:idx] + "?[REDACTED]"
+	} else {
+		sanitizedURL = apiURL
+	}
+	log.Printf("[HTTP] %s %s - Status: %d", method, sanitizedURL, resp.StatusCode)
 	return resp, nil
 }