Skip to content

fix(security): remediate SonarQube vulnerabilities in discussions.go#80

Open
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
devin/1770237075-sonarqube-discussions-fixes
Open

fix(security): remediate SonarQube vulnerabilities in discussions.go#80
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
devin/1770237075-sonarqube-discussions-fixes

Conversation

@devin-ai-integration
Copy link

@devin-ai-integration devin-ai-integration bot commented Feb 4, 2026

Closes: N/A (SonarQube remediation task)

Summary

This PR addresses 6 CRITICAL SonarQube issues in pkg/github/discussions.go:

S1192 - Duplicated string literals (4 issues):

  • Extracted constants for repeated strings: descRepositoryOwner, descRepositoryName, errFailedToGetGQLClient, labelCategoryFormat

S3776 - Cognitive complexity (2 issues):

  • ListDiscussions: Reduced complexity from 27 to ~10 by extracting discussionNode type, mapDiscussionNodesToIssues, fetchDiscussionsWithCategory, and fetchDiscussionsWithoutCategory helper functions
  • ListDiscussionCategories: Reduced complexity from 22 to ~10 by extracting validatePaginationParams helper function

Human Review Checklist

  • Verify GraphQL queries work correctly with the extracted discussionNode type (the graphql struct tags are preserved)
  • Confirm pagination validation error messages are unchanged
  • Check that the refactored ListDiscussions maintains identical behavior for both with-category and without-category code paths

Testing

  • All existing tests pass (./script/test)
  • Lint checks pass (./script/lint)

Link to Devin run: https://app.devin.ai/sessions/55dea36d2ba74088b5f0faccc32d4dbc
Requested by: @parkerduff


Open with Devin

- S1192: Define constants for duplicated string literals:
  - descRepositoryOwner for 'Repository owner' (4 occurrences)
  - descRepositoryName for 'Repository name' (4 occurrences)
  - errFailedToGetGQLClient for error message (4 occurrences)
  - labelCategoryFormat for 'category:%s' (3 occurrences)

- S3776: Reduce cognitive complexity in ListDiscussions (27 -> ~10):
  - Extract discussionNode type for reuse
  - Extract mapDiscussionNodesToIssues helper function
  - Extract fetchDiscussionsWithCategory helper function
  - Extract fetchDiscussionsWithoutCategory helper function

- S3776: Reduce cognitive complexity in ListDiscussionCategories (22 -> ~10):
  - Extract validatePaginationParams helper function

Co-Authored-By: parker.duff@codeium.com <pwjduff@gmail.com>
@devin-ai-integration
Copy link
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 4, 2026

Copy link
Author

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 3 additional findings in Devin Review.

Open in Devin Review

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 ListDiscussionCategories pagination parameters are validated but silently ignored in the GraphQL query

The ListDiscussionCategories function accepts pagination parameters (first, last, after, before), decodes them from the request, and validates them via the newly extracted validatePaginationParams function, but the GraphQL query at line 361 hardcodes discussionCategories(first: 100) and never uses any of the user-supplied pagination values.

Root Cause and Impact

The params.First, params.Last, params.After, and params.Before fields are decoded at pkg/github/discussions.go:334-341 and validated at line 346, but the GraphQL query struct tag at line 361 is hardcoded:

`graphql:"discussionCategories(first: 100)"`

And the vars map at lines 364-367 only includes owner and repo:

vars := map[string]interface{}{
    "owner": githubv4.String(params.Owner),
    "repo":  githubv4.String(params.Repo),
}

Impact: Users who pass first, last, after, or before parameters will receive no error (validation passes), but their pagination preferences are completely ignored. The query always returns the first 100 categories. This is particularly misleading now that validatePaginationParams has been extracted as a shared utility, suggesting pagination is properly implemented.

(Refers to line 361)

Prompt for agents
In pkg/github/discussions.go, the ListDiscussionCategories function (starting around line 301) accepts first, last, after, and before pagination parameters, validates them at line 346, but the GraphQL query at line 361 hardcodes `discussionCategories(first: 100)` and the vars map at lines 364-367 doesn't include any pagination variables.

To fix this, you need to:
1. Dynamically construct the GraphQL query struct tag based on which pagination parameters are provided (first/last, after/before), or use separate query structs for different pagination combinations (similar to how fetchDiscussionsWithCategory vs fetchDiscussionsWithoutCategory work).
2. Include the pagination variables in the vars map passed to client.Query.
3. If no pagination parameters are provided, default to first: 100 as the current behavior.

Alternatively, if pagination is not intended to be supported yet, remove the first/last/after/before parameters from the tool schema definition (lines 316-331) and remove the validatePaginationParams call.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants

Comments