Skip to content

fix(access_groups): derive attached teams from the team table and reject unknown team ids - #39218

Merged
ryan-crabbe-berri merged 2 commits into
litellm_internal_stagingfrom
litellm_lit_6593_access_group_attached_teams
Sep 3, 2026
Merged

ryan-crabbe-berri merged 2 commits into
litellm_internal_stagingfrom
litellm_lit_6593_access_group_attached_teams

Conversation

@ryan-crabbe-berri

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • Access group "Attached Teams" is read off a mirror column that drifts
  • Teams attached before the mirror existed never show up (undercount)
  • Deleted or mistyped team ids stay in the list forever (ghosts)
  • Nothing stops an admin from saving a team id that does not exist (Pylon 8024)

How it solves it:

  • GET list and detail derive the teams from the team table, unioned with the column, ghosts dropped
  • PUT computes its add and remove deltas from that same derived set
  • POST and PUT return 400 Unknown team ids: ... for ids that resolve to no team

User Flow

Before: an admin opens an access group and the Attached Teams card is wrong in both directions

  1. They open http://localhost:4000/ui/?page=access-groups and click a group that three teams use
  2. The Attached Teams card lists two real team ids plus team-that-does-not-exist-8024, count 3, and the third real team is missing
  3. They send GET /v1/access_group/<id> and get the same three ids back
  4. They send PUT /v1/access_group/<id> with the real team plus the ghost id and get 200, so the ghost is saved again
  5. Keys on the missing team can still use the group's models, so the page contradicts what the gateway enforces

After: the card shows the teams that really carry the group, and bad ids are rejected

  1. They open http://localhost:4000/ui/?page=access-groups and click the same group
  2. The Attached Teams card lists the three real team ids, count 3, and no ghost
  3. They send GET /v1/access_group/<id> and get the same three real ids back
  4. They send PUT /v1/access_group/<id> with the real team plus the ghost id and get 400 {"detail": "Unknown team ids: team-that-does-not-exist-8024"}, nothing changes
  5. Removing the third team from the list via PUT detaches it even though the mirror never listed it

Relevant issues

Linear ticket

Resolves LIT-6593

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • The handful of test files covering my change pass locally, e.g. uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*, make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more
  • My PR passes all required CI/CD checks (e.g., lint, schema.d.ts sync check, etc.)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Screenshots / Proof of Fix

Setup, worktree proxy on :4592 against the shared dev Postgres, dashboard dev server on :3592. Access group 1936eb56-aeb4-4932-af4e-86012582d24f has three teams attached (lit-6593-team-alpha 92851a2d..., lit-6593-team-beta 0580ab49..., lit-6593-team-gamma 4766b850...). Drift was simulated the way old data looks in the field: the group's stored team column was set to alpha, beta and team-that-does-not-exist-8024, so gamma carries the group but is missing from the column and the ghost id is present

P=http://localhost:4592; K="Authorization: Bearer sk-1234"; AG=1936eb56-aeb4-4932-af4e-86012582d24f

Before (97dbd8e)

GET detail

  1. curl -s $P/v1/access_group/$AG -H "$K" | jq '{access_group_name, assigned_team_ids}'
  2. {"access_group_name":"lit-6592-platform-tools","assigned_team_ids":["92851a2d-c6db-41ac-ae6e-f8e37786f82f","0580ab49-3559-46c6-822e-13b20ce4c545","team-that-does-not-exist-8024"]}

GET list

  1. curl -s $P/v1/access_group -H "$K" | jq '.[] | select(.access_group_id=="'$AG'") | .assigned_team_ids'
  2. ["92851a2d-c6db-41ac-ae6e-f8e37786f82f","0580ab49-3559-46c6-822e-13b20ce4c545","team-that-does-not-exist-8024"]

POST with a team id that does not exist

  1. curl -s -w 'HTTP %{http_code}\n' -X POST $P/v1/access_group -H "$K" -H 'Content-Type: application/json' -d '{"access_group_name":"lit-6593-ghost-probe","assigned_team_ids":["team-that-does-not-exist-8024"]}'
  2. HTTP 201 with {"access_group_id":"87be2b8a-6798-4ec7-a319-2242479ed824","assigned_team_ids":["team-that-does-not-exist-8024"]}

PUT with a team id that does not exist

  1. curl -s -w 'HTTP %{http_code}\n' -X PUT $P/v1/access_group/$AG -H "$K" -H 'Content-Type: application/json' -d '{"assigned_team_ids":["92851a2d-c6db-41ac-ae6e-f8e37786f82f","team-that-does-not-exist-8024"]}'
  2. HTTP 200 with {"access_group_id":"1936eb56-aeb4-4932-af4e-86012582d24f","assigned_team_ids":["92851a2d-c6db-41ac-ae6e-f8e37786f82f","team-that-does-not-exist-8024"]}

PUT that drops gamma, then read gamma back

  1. curl -s -w 'HTTP %{http_code}\n' -X PUT $P/v1/access_group/$AG -H "$K" -H 'Content-Type: application/json' -d '{"assigned_team_ids":["92851a2d-c6db-41ac-ae6e-f8e37786f82f","0580ab49-3559-46c6-822e-13b20ce4c545"]}'
  2. HTTP 200
  3. curl -s "$P/team/info?team_id=4766b850-d699-4bb9-bcb4-63349d324b26" -H "$K" | jq -c '.team_info | {team_alias, access_group_ids}'
  4. {"team_alias":"lit-6593-team-gamma","access_group_ids":["1936eb56-aeb4-4932-af4e-86012582d24f"]}, gamma still carries the group it was just removed from

Admin UI

  1. Open http://localhost:3592/access-groups and click 1936eb56-aeb4-4932-af4e-86012582d24f
  2. Attached Teams shows alpha, beta and the ghost id; gamma is missing

before

After (c14cf9d)

The probe group from Before was deleted and the drifted column re-seeded to the same alpha, beta, ghost state before this run. The Admin UI screenshot on each side was taken before that side's PUT case

GET detail

  1. curl -s $P/v1/access_group/$AG -H "$K" | jq '{access_group_name, assigned_team_ids}'
  2. {"access_group_name":"lit-6592-platform-tools","assigned_team_ids":["92851a2d-c6db-41ac-ae6e-f8e37786f82f","0580ab49-3559-46c6-822e-13b20ce4c545","4766b850-d699-4bb9-bcb4-63349d324b26"]}

GET list

  1. curl -s $P/v1/access_group -H "$K" | jq '.[] | select(.access_group_id=="'$AG'") | .assigned_team_ids'
  2. ["92851a2d-c6db-41ac-ae6e-f8e37786f82f","0580ab49-3559-46c6-822e-13b20ce4c545","4766b850-d699-4bb9-bcb4-63349d324b26"]

POST with a team id that does not exist

  1. curl -s -w 'HTTP %{http_code}\n' -X POST $P/v1/access_group -H "$K" -H 'Content-Type: application/json' -d '{"access_group_name":"lit-6593-ghost-probe","assigned_team_ids":["team-that-does-not-exist-8024"]}'
  2. HTTP 400 with {"detail":"Unknown team ids: team-that-does-not-exist-8024"}

PUT with a team id that does not exist

  1. curl -s -w 'HTTP %{http_code}\n' -X PUT $P/v1/access_group/$AG -H "$K" -H 'Content-Type: application/json' -d '{"assigned_team_ids":["92851a2d-c6db-41ac-ae6e-f8e37786f82f","team-that-does-not-exist-8024"]}'
  2. HTTP 400 with {"detail":"Unknown team ids: team-that-does-not-exist-8024"}

PUT that drops gamma, then read gamma back

  1. curl -s -w 'HTTP %{http_code}\n' -X PUT $P/v1/access_group/$AG -H "$K" -H 'Content-Type: application/json' -d '{"assigned_team_ids":["92851a2d-c6db-41ac-ae6e-f8e37786f82f","0580ab49-3559-46c6-822e-13b20ce4c545"]}'
  2. HTTP 200
  3. curl -s "$P/team/info?team_id=4766b850-d699-4bb9-bcb4-63349d324b26" -H "$K" | jq -c '.team_info | {team_alias, access_group_ids}'
  4. {"team_alias":"lit-6593-team-gamma","access_group_ids":[]}, gamma is detached

Admin UI

  1. Open http://localhost:3592/access-groups and click 1936eb56-aeb4-4932-af4e-86012582d24f
  2. Attached Teams shows alpha, beta and gamma; the ghost id is gone

after

Type

🐛 Bug Fix

Caveats (if any)

Severe

  • Behavior change on POST and PUT /v1/access_group: a team id that resolves to no team used to be stored silently (201/200), it is now a 400. Any automation that seeds groups before the teams exist has to create the teams first

Low

  • The stored assigned_team_ids column is not backfilled; it self-heals on the next PUT that includes assigned_team_ids, and reads no longer depend on it
  • assigned_key_ids has the same mirror shape and was left alone; that is a separate ticket if it bites
  • The Attached Teams card still shows raw team ids rather than aliases, unchanged from before

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

…ect unknown team ids

GET /v1/access_group and GET /v1/access_group/{id} (and the /v1/unified_access_group aliases) used to
return the assigned_team_ids column verbatim. That column is a denormalized mirror of
LiteLLM_TeamTable.access_group_ids and can be stale or hold ids of teams that no longer exist, so the
Attached Teams view drifted from reality.

The read path now runs one team find_many per request, unioning teams whose access_group_ids carry any
group in the response with teams listed in the stored columns. Only real team rows come back, so ghost
ids drop out and teams the mirror missed are added. The stored order is kept for ids that survive and
newly discovered teams are appended.

Create and update now resolve the requested assigned_team_ids inside the transaction and answer 400
with the missing ids before anything is written, instead of silently storing ids that point nowhere.

Refs LIT-6593

Claude-Session: https://claude.ai/code/session_01QvQzYztinxj8ZuD5YxbVdL
Team membership deltas on PUT now start from the teams that really carry
the group, so a team the mirror column missed can be detached. Read
endpoints go through a typed TeamRepository instead of the untyped db
handle, and the where clause always carries both OR arms.

Claude-Session: https://claude.ai/code/session_01QvQzYztinxj8ZuD5YxbVdL
@codspeed

codspeed Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing litellm_lit_6593_access_group_attached_teams (c14cf9d) with litellm_internal_staging (9c417ba)1

Open in CodSpeed

Footnotes

  1. No successful run was found on litellm_internal_staging (346efa0) during the generation of this report, so 9c417ba was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

@greptile-apps

greptile-apps Bot commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR makes the team table authoritative when reading and updating access-group team attachments and rejects references to teams that do not exist.

  • Derives attached teams from stored IDs and actual team records while dropping ghost IDs.
  • Uses the derived attachment set when calculating update deltas.
  • Validates assigned team IDs during access-group creation and updates.
  • Adds focused tests for drifted mirrors, unknown IDs, and synchronization behavior.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; both previously reported issues were withdrawn after the contract change was confirmed intentional and the new database reads were confirmed to be outside the critical LLM request path.

Important Files Changed

Filename Overview
litellm/proxy/management_endpoints/access_group_endpoints.py Reconciles access-group team attachments against team records and validates referenced teams before writes; no eligible blocking issue remains.
litellm/repositories/table_repositories.py Adds the standard repository wrapper for the team table.
tests/test_litellm/proxy/management_endpoints/test_access_group_endpoints.py Expands coverage for derived attachments, ghost rejection, and synchronization of previously unmirrored teams.

Reviews (2): Last reviewed commit: "fix(access_groups): reconcile update del..." | Re-trigger Greptile

Comment thread litellm/proxy/management_endpoints/access_group_endpoints.py
Comment thread litellm/proxy/management_endpoints/access_group_endpoints.py
@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@ryan-crabbe-berri

Copy link
Copy Markdown
Contributor Author

@greptileai please re-review: both earlier findings were withdrawn, so the confidence score should be refreshed against the tip

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.

2 participants