feat(github): support team hierarchy in GH_TEAM_ALLOWLIST by hussein-mimi · Pull Request #6365 · runatlantis/atlantis

hussein-mimi · 2026-04-05T13:06:44Z

Summary

When a parent team is added to ATLANTIS_GH_TEAM_ALLOWLIST, users who are members of any descendant (child/grandchild/etc.) team are now correctly authorized, instead of being rejected.

Before: User in child-team → allowlist has parent-team → ❌ rejected
After: User in child-team → allowlist has parent-team → ✅ authorized

How it works

Added GetChildTeams(logger, repo, teamSlug) to the GitHub client — queries the GitHub GraphQL API for direct child teams of a given team slug (with pagination).
In checkUserPermissions, after the fast path (direct team membership, zero extra API calls) fails, a slow path kicks in:
1. For each allowlisted team that grants the requested command, recursively fetch all descendant teams via GetChildTeams (up to 20 levels deep to prevent infinite loops).
2. Check if the user's direct teams appear in this expanded set.
Non-GitHub VCS clients are unaffected — the expansion uses a duck-typed childTeamFetcher interface, so no changes to the shared Client interface or any other VCS provider.

Test plan

TestClient_GetTeamNamesForUser — existing test, still passes unchanged
TestClient_GetChildTeams — new test verifying GetChildTeams returns direct children from a mocked GraphQL response
go build ./server/events/... — clean build
Manual test: add a parent team to ATLANTIS_GH_TEAM_ALLOWLIST, comment as a user who is only in a child team, verify they are now authorized

🤖 Generated with Claude Code

Copilot

Pull request overview

This PR adds support for GitHub team hierarchy in the Atlantis allowlist. When a parent team is added to ATLANTIS_GH_TEAM_ALLOWLIST, users who are members of descendant (child/grandchild/etc.) teams are now correctly authorized. The implementation uses a duck-typed interface to support team hierarchies for VCS clients that support them (like GitHub), while remaining compatible with VCS clients that don't support this feature.

Changes:

Added GetChildTeams() method to the GitHub client that queries the GraphQL API for direct child teams with pagination support
Added childTeamFetcher interface for VCS clients supporting team hierarchies
Added fetchDescendantTeams() recursive function with depth limiting to expand teams to all descendants
Updated checkUserPermissions() with a two-path approach: fast path for direct membership, slow path for hierarchical expansion
Added test for GetChildTeams() method

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
server/events/vcs/github/client.go	Added `GetChildTeams()` method to query GitHub's GraphQL API for direct child teams with pagination
server/events/vcs/github/client_test.go	Added `TestClient_GetChildTeams()` to verify the method correctly parses GraphQL responses
server/events/command_runner.go	Added `childTeamFetcher` interface, `fetchDescendantTeams()` function, and updated `checkUserPermissions()` to support team hierarchy expansion with a two-path authorization check

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

When a parent team is added to ATLANTIS_GH_TEAM_ALLOWLIST, users who are members of any descendant (child/grandchild) team are now correctly authorized, instead of being rejected. The fix adds GetChildTeams to the GitHub client, which fetches direct child teams via GraphQL. In checkUserPermissions, after the fast-path direct-membership check fails, each allowlisted team is expanded to all its descendants (up to 20 levels deep) using recursive GetChildTeams calls. The user's direct teams are then checked against this expanded set. Non-GitHub VCS clients are unaffected. Fixes runatlantis#6107 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: hussein-mimi <hussein.mimi@harri.com>

…ions hierarchy Addresses the missing test coverage flagged by Copilot and code review: - TestFetchDescendantTeams: leaf team, single/multi-level nesting, maxDepth cutoff at 0 and 1, error propagation at root, and soft-fail on recursive errors while sibling subtrees continue - TestCheckUserPermissions: nil checker, direct member fast path, non-member rejection without hierarchy support, and table-driven slow-path cases (child team allowed, grandchild team allowed, unrelated team rejected, wrong command rejected, wildcard rule with no-team user) Also adds childTeamVCSClient test helper that satisfies both vcs.Client and childTeamFetcher, enabling the slow path to be exercised without a real VCS connection. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: hussein-mimi <hussein.mimi@harri.com>

…ntedGithubClient The team hierarchy slow path in checkUserPermissions asserts c.VCSClient against childTeamFetcher, but c.VCSClient is always a *ClientProxy wrapping an *InstrumentedGithubClient. Neither type implemented GetChildTeams, so the assertion always failed and hierarchy expansion was silently skipped. Add GetChildTeams to both ClientProxy and InstrumentedGithubClient so the interface is satisfied and calls are correctly delegated to the underlying *github.Client. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: hussein-mimi <hussein.mimi@harri.com>

When a user belongs to a child team of an allowlisted team, the global checkUserPermissions passes via the hierarchy slow path, but the per-project filter in buildAllCommandsByCfg/buildProjectCommandCtxs only does a direct team membership check. This caused child-team members to always see "Ran Plan for 0 projects:" even though the command-level check allowed them through. Fix by changing checkUserPermissions to accept *models.User and appending the matched allowlisted parent team to user.Teams when a hierarchy match is found. This ensures subsequent per-project allowlist checks (which use direct membership) also pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: hussein-mimi <hussein.mimi@harri.com>

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

…on-GitHub providers Addresses PR review feedback: instead of using anonymous types and type assertions in the proxy and instrumented client, GetChildTeams is now a first-class method on the Client interface. All non-GitHub VCS providers return nil, nil as they don't support team hierarchies. Signed-off-by: hussein-mimi <hussein.mimi@harri.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

The debian-security repo updated openssh to u9 which requires a matching openssh-client version, causing a dependency conflict with the previously pinned u7 version. Signed-off-by: hussein-mimi <hussein.mimi@harri.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

server/events/vcs/client.go:1

The PR description says the hierarchy support is implemented via a duck-typed childTeamFetcher without changing the shared Client interface, but this diff adds GetChildTeams to vcs.Client, forcing all VCS providers (and the proxy/mocks) to implement it. If the intent is truly duck-typing, keep GetChildTeams off vcs.Client and only implement it on the GitHub client (and adjust call sites to type-assert to the extra interface); otherwise, update the PR description to reflect this breaking interface change.

// Copyright 2017 HootSuite Media Inc.

…missions Since GetChildTeams is now on the vcs.Client interface (per reviewer request), the type assertion c.VCSClient.(childTeamFetcher) always succeeds, causing the slow path to execute for all VCS providers even when they just return nil. Remove the assertion and call c.VCSClient directly; non-GitHub providers return nil from GetChildTeams so fetchDescendantTeams produces empty results and the loop is a no-op. Keep childTeamFetcher as a narrow interface for fetchDescendantTeams so tests can pass a mock without implementing all of vcs.Client. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: hussein-mimi <hussein.mimi@harri.com>

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Add a 'multiple pages' subtest to TestClient_GetChildTeams that verifies cursor-based pagination works correctly: the first GraphQL request has no cursor, the second includes the endCursor from the first page, and results from both pages are accumulated. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: hussein-mimi <hussein.mimi@harri.com>

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Verify that a cyclic hierarchy (a -> b -> a) terminates and produces deduplicated results thanks to the visited set. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: hussein-mimi <hussein.mimi@harri.com>

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

hussein-mimi · 2026-04-16T07:52:12Z

@lukemassa can you take a look ?

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

GetChildTeams is already on vcs.Client, so childTeamFetcher was a strict subset of it — the type assertion always succeeded and the interface served no selection purpose. fetchDescendantTeams now takes vcs.Client directly, and the separate mockChildTeamFetcher test double is merged into childTeamVCSClient so there is one test helper instead of two. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: hussein-mimi <hussein.mimi@harri.com>

lukemassa

I left a small comment, but otherwise this looks good to me!

lukemassa · 2026-05-07T16:55:27Z

+// When a match is found via hierarchy, the matched allowlisted parent team is appended to
+// user.Teams so that subsequent per-project allowlist checks (which use direct membership
+// only) also pass.
+func (c *DefaultCommandRunner) checkUserPermissions(repo models.Repo, user *models.User, cmdName string) (bool, error) {


Why was the signature changed to make user a pointer type? It seems like all callers of this function are taking a pointer at the call site, and the user is immediately dereferenced below, what is that gaining?

Copilot AI review requested due to automatic review settings April 5, 2026 13:06

dosubot Bot added feature New functionality/enhancement go Pull requests that update Go code provider/github labels Apr 5, 2026

Copilot started reviewing on behalf of hussein-mimi April 5, 2026 13:07 View session

Copilot AI reviewed Apr 5, 2026

View reviewed changes

Comment thread server/events/command_runner.go Outdated

hussein-mimi force-pushed the feat/github-team-hierarchy-allowlist branch from 73d098f to bb0b23d Compare April 7, 2026 16:39

github-actions Bot added the website label Apr 7, 2026

hussein-mimi force-pushed the feat/github-team-hierarchy-allowlist branch from bb0b23d to b92451e Compare April 7, 2026 16:42

hussein-mimi requested a review from Copilot April 8, 2026 06:50

Copilot started reviewing on behalf of hussein-mimi April 8, 2026 06:50 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

hussein-mimi force-pushed the feat/github-team-hierarchy-allowlist branch from 6d9cb59 to 6dab523 Compare April 9, 2026 08:29

hussein-mimi marked this pull request as draft April 9, 2026 12:51

hussein-mimi marked this pull request as ready for review April 9, 2026 12:54

hussein-mimi mentioned this pull request Apr 9, 2026

ATLANTIS_GH_TEAM_ALLOWLIST does not support nested team membership #6107

Open

1 task

hussein-mimi and others added 4 commits April 9, 2026 16:00

hussein-mimi force-pushed the feat/github-team-hierarchy-allowlist branch from 77c3ffd to f57331f Compare April 9, 2026 13:00

hussein-mimi requested a review from Copilot April 9, 2026 13:01

Copilot started reviewing on behalf of hussein-mimi April 9, 2026 13:02 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

hussein-mimi requested a review from Copilot April 9, 2026 13:20

Copilot started reviewing on behalf of hussein-mimi April 9, 2026 13:21 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

lukemassa reviewed Apr 11, 2026

View reviewed changes

Comment thread server/events/vcs/proxy.go

hussein-mimi and others added 2 commits April 12, 2026 11:22

Copilot started reviewing on behalf of hussein-mimi April 12, 2026 09:11 View session

Copilot AI reviewed Apr 12, 2026

View reviewed changes

Comment thread server/events/command_runner.go Outdated

Comment thread server/events/vcs/github/client_test.go Outdated

hussein-mimi requested a review from Copilot April 12, 2026 09:32

Copilot started reviewing on behalf of hussein-mimi April 12, 2026 09:32 View session

Copilot AI reviewed Apr 12, 2026

View reviewed changes

Comment thread server/events/command_runner.go

Comment thread server/events/vcs/github/client_test.go Outdated

Comment thread server/events/vcs/github/client_test.go Outdated

hussein-mimi requested a review from Copilot April 12, 2026 09:53

Copilot started reviewing on behalf of hussein-mimi April 12, 2026 09:54 View session

Copilot AI reviewed Apr 12, 2026

View reviewed changes

Comment thread server/events/command_runner.go

Comment thread server/events/command_runner.go

Comment thread server/events/command_runner_internal_test.go

test: add cycle detection test for fetchDescendantTeams

4133a20

Verify that a cyclic hierarchy (a -> b -> a) terminates and produces deduplicated results thanks to the visited set. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: hussein-mimi <hussein.mimi@harri.com>

hussein-mimi requested a review from Copilot April 12, 2026 10:06

Copilot started reviewing on behalf of hussein-mimi April 12, 2026 10:06 View session

Copilot AI reviewed Apr 12, 2026

View reviewed changes

Comment thread server/events/command_runner.go

hussein-mimi added 2 commits April 13, 2026 10:14

Merge branch 'main' into feat/github-team-hierarchy-allowlist

d5d19bf

Merge branch 'main' into feat/github-team-hierarchy-allowlist

dfca196

Merge branch 'main' into feat/github-team-hierarchy-allowlist

6cfec46

hussein-mimi requested a review from Copilot April 19, 2026 15:57

Copilot started reviewing on behalf of hussein-mimi April 19, 2026 15:58 View session

Copilot AI reviewed Apr 19, 2026

View reviewed changes

Comment thread server/events/command_runner.go

hussein-mimi and others added 7 commits April 21, 2026 18:22

Merge branch 'main' into feat/github-team-hierarchy-allowlist

46279b9

Merge branch 'main' into feat/github-team-hierarchy-allowlist

30a53ea

Merge branch 'main' into feat/github-team-hierarchy-allowlist

44bb779

Merge branch 'main' into feat/github-team-hierarchy-allowlist

f5efeaa

Merge branch 'main' into feat/github-team-hierarchy-allowlist

3ce40a6

Merge branch 'main' into feat/github-team-hierarchy-allowlist

fa51f2d

lukemassa approved these changes May 7, 2026

View reviewed changes

dosubot Bot added the lgtm This PR has been approved by a maintainer label May 7, 2026

Conversation

hussein-mimi commented Apr 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

How it works

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

hussein-mimi commented Apr 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

lukemassa left a comment

Choose a reason for hiding this comment

Uh oh!

lukemassa May 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hussein-mimi commented Apr 5, 2026 •

edited

Loading