Skip to content

fix(security): remediate SonarQube vulnerabilities in notifications.go#85

Open
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
devin/1770237105-sonarqube-remediation
Open

fix(security): remediate SonarQube vulnerabilities in notifications.go#85
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
devin/1770237105-sonarqube-remediation

Conversation

@devin-ai-integration
Copy link

Closes: N/A (SonarQube remediation task)

Summary

This PR remediates 8 high-severity SonarQube issues in pkg/github/notifications.go:

Rule Count Description
S1192 3 Duplicated string literals
S3776 5 Cognitive complexity too high

Changes

S1192 - Duplicated String Literals: Defined package-level constants for repeated error message format strings:

  • errFailedToGetGitHubClient (was duplicated 6 times)
  • errFailedToReadResponseBody (was duplicated 4 times)
  • errFailedToMarshalResponse (was duplicated 4 times)

S3776 - Cognitive Complexity: Extracted helper functions from complex handlers:

  • ListNotifications: Extracted extractListNotificationsParams, buildNotificationListOptions, fetchNotifications, handleNotificationResponse
  • DismissNotification: Extracted dismissNotificationByState, handleDismissResponse
  • MarkAllNotificationsRead: Extracted extractMarkAllNotificationsReadParams, parseLastReadAtTime, markNotificationsRead, handleMarkAllReadResponse
  • ManageNotificationSubscription: Extracted executeThreadSubscriptionAction, handleSubscriptionResponse
  • ManageRepositoryNotificationSubscription: Extracted executeRepoSubscriptionAction, handleRepoSubscriptionResponse

Tradeoffs

  • Added //nolint:revive,staticcheck directives for user-facing error messages that intentionally use capitalization and punctuation for readability. These messages are returned via mcp.NewToolResultError() to end users.

Review Checklist

  • Verify error handling flow in helper functions matches original behavior (especially resp == nil checks in DismissNotification and ManageNotificationSubscription)
  • Confirm the nolint directives are acceptable for user-facing error messages

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

- S1192: Define constants for duplicated string literals
  - errFailedToGetGitHubClient (6 occurrences)
  - errFailedToReadResponseBody (4 occurrences)
  - errFailedToMarshalResponse (4 occurrences)

- S3776: Reduce cognitive complexity by extracting helper functions
  - ListNotifications: Extract extractListNotificationsParams, buildNotificationListOptions, fetchNotifications, handleNotificationResponse
  - DismissNotification: Extract dismissNotificationByState, handleDismissResponse
  - MarkAllNotificationsRead: Extract extractMarkAllNotificationsReadParams, parseLastReadAtTime, markNotificationsRead, handleMarkAllReadResponse
  - ManageNotificationSubscription: Extract executeThreadSubscriptionAction, handleSubscriptionResponse
  - ManageRepositoryNotificationSubscription: Extract executeRepoSubscriptionAction, handleRepoSubscriptionResponse

All tests pass and lint checks pass.

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

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