Bug/issue-903 - #1010
Open
madkoo wants to merge 10 commits into
Open
Conversation
…s in settings.json
…erty in settings.json
…to main-enterprise
…to main-enterprise
…ings into main-enterprise
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the teams plugin to identify “Security Manager” teams via the GitHub Organization Roles API (instead of the deprecated security managers endpoint), improves team name/slug normalization to reduce churn, and updates required GitHub App permissions + docs, with expanded unit/integration test coverage.
Changes:
- Refactor security-manager team detection to use
/orgs/{org}/organization-rolesand/orgs/{org}/organization-roles/{role_id}/teams, with graceful failure behavior. - Normalize team identifiers (name/slug) for matching and for
getByNamelookups. - Add required
organization_custom_roles: readpermission and document it; expand tests to cover the new behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| lib/plugins/teams.js | Uses org roles API to discover security-manager teams, adds normalization helpers, and introduces deletion-skipping behavior on discovery failures. |
| test/unit/lib/plugins/teams.test.js | Adds/updates unit tests for security-manager exclusion, failure modes, and normalization behavior. |
| test/integration/plugins/teams.test.js | Extends integration test to mock org roles API calls and ensure security-manager teams aren’t removed. |
| docs/deploy.md | Documents the new “Custom organization roles: Read-only” permission requirement. |
| app.yml | Adds organization_custom_roles: read to app default permissions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
42
to
+46
| this.log.debug(`Response from the call is ${JSON.stringify(resp)}`) | ||
| return teams.filter(team => !resp.some(sec => sec.name === team.name)) | ||
| const securityManagerTeams = this.toArray(resp, 'teams') | ||
| const securityManagerTeamIdentifiers = new Set(securityManagerTeams.flatMap(team => [team.slug, team.name].map(name => this.normalizeTeamIdentifier(name))).filter(Boolean)) | ||
|
|
||
| return teams.filter(team => !this.isSecurityManagerTeam(team, securityManagerTeamIdentifiers)) |
decyjphr
added a commit
that referenced
this pull request
Aug 4, 2026
…r-1010-security-manager Modernize security manager team handling (incorporate PR #1010)
vish-dawange
pushed a commit
to vish-dawange/safe-settings
that referenced
this pull request
Aug 24, 2026
…ager team handling Bring PR github-community-projects#1010 (bug/issue-903) into this branch. The comprehensive teams.js here still used the deprecated GET /orgs/{org}/security-managers endpoint, so the security-manager modernization was not yet incorporated. - teams.js: identify security manager teams via the organization roles API (GET /orgs/{org}/organization-roles and .../{role_id}/teams) instead of the deprecated security-managers endpoint. Adapted to this branch's non-`rest` Octokit client convention (this.github.repos/teams.*). - Add team name/slug and role name normalization helpers so configured names match existing slugs without add/remove churn. - Add skipTeamDeletion guard: if security manager discovery fails, keep repository teams unchanged instead of deleting them. - app.yml already grants organization_custom_roles (write), so no permission change needed; document the org "Custom organization roles" permission in docs/deploy.md. - Port unit + integration test coverage for the new behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 64c2ef45-ed6c-4756-9ec6-58a35797dc0e
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request enhances the handling of GitHub organization "Security Manager" teams by updating permissions, refactoring the
teamsplugin to use the new organization roles API, and improving normalization and matching of team names. It also adds robust error handling and expands test coverage to ensure correct behavior when working with security manager teams.Key changes include:
Security Manager Team Handling:
teamsplugin to use the new organization roles API (/orgs/{org}/organization-rolesand/orgs/{org}/organization-roles/{role_id}/teams) to identify and exclude security manager teams from repository team management, replacing the deprecated security managers endpoint. The plugin now normalizes team names and slugs for accurate matching and skips deletion of security manager teams if role lookups fail gracefully.Permissions and Documentation:
organization_custom_roles: readpermission inapp.ymland updated deployment documentation to reflect the new required permission for accessing organization roles.Team Name Normalization:
teamsplugin to ensure consistent matching and prevent unnecessary add/remove operations due to formatting differences.Testing Enhancements:
These changes ensure that security manager teams are properly identified and protected from accidental removal, while keeping the codebase up to date with GitHub API changes.