BCH 1033: Add support for defining security team name #4
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for defining a security team name in the Dependabot plugin configuration, enabling the collection of security team members for policy evaluation. The changes extend the plugin to fetch team membership data when a security team is configured.
- Adds
SecurityTeamNameconfiguration field to specify which GitHub team represents the security team - Implements
FetchSecurityTeamMembersfunction to retrieve team member data from GitHub API - Updates data structure to include security team members alongside Dependabot alerts for policy evaluation
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| main.go | Adds SecurityTeamName config field, implements team member fetching, and updates data structure to include security team members |
| main_integration_test.go | Adds integration test for the new FetchSecurityTeamMembers functionality and reorganizes imports |
| Makefile | Adds build tooling with OPA CLI validation and basic make targets for building and cleaning |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| } | ||
|
|
||
| func (l *DependabotPlugin) FetchSecurityTeamMembers(ctx context.Context) ([]*github.User, error) { | ||
| members, _, err := l.githubClient.Teams.ListTeamMembersBySlug(ctx, *l.config.Organization, *l.config.SecurityTeamName, nil) |
There was a problem hiding this comment.
Potential nil pointer dereference. The method dereferences l.config.Organization and l.config.SecurityTeamName without checking if they are nil, which could cause a panic if either field is not set.
| repochan, errchan := l.FetchRepositories(ctx) | ||
|
|
||
| var securityTeamMembers []*github.User | ||
| if l.config.SecurityTeamName != nil && *l.config.SecurityTeamName != "" { |
There was a problem hiding this comment.
Inconsistent nil checking. This code checks both SecurityTeamName and Organization for nil, but the FetchSecurityTeamMembers method at line 116 only checks SecurityTeamName for nil while still dereferencing Organization without validation.
| if l.config.SecurityTeamName != nil && *l.config.SecurityTeamName != "" { | |
| if l.config.SecurityTeamName != nil && *l.config.SecurityTeamName != "" && l.config.Organization != nil && *l.config.Organization != "" { |
No description provided.