Skip to content

fix(policies): reject non-existent team/key/model scope entries on attachment create - #32131

Merged
ryan-crabbe-berri merged 3 commits into
litellm_internal_stagingfrom
litellm_lit_4199_cannot_select_team_in_attachment
Jul 4, 2026
Merged

fix(policies): reject non-existent team/key/model scope entries on attachment create#32131
ryan-crabbe-berri merged 3 commits into
litellm_internal_stagingfrom
litellm_lit_4199_cannot_select_team_in_attachment

Conversation

@ryan-crabbe-berri

@ryan-crabbe-berri ryan-crabbe-berri commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Relevant issues

Linear ticket

Resolves LIT-4199

Pre-Submission checklist

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

  • I have added meaningful tests
  • My PR passes all CI/CD checks (e.g., lint, format, unit tests)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

Screenshots / Proof of Fix

Backend, against a live proxy on localhost:4000. Before this change every one of these returned 200 and persisted the bogus value

KEY=sk-1234; BASE=http://localhost:4000
# a policy to attach (lands in "production" immediately)
curl -s -X POST "$BASE/policies" -H "Authorization: Bearer $KEY" -H 'Content-Type: application/json' \
  -d '{"policy_name":"repro-lit4199","guardrails_add":[],"guardrails_remove":[]}' -o /dev/null -w "create policy: %{http_code}\n"

# 1. concrete team that does not exist -> now 400
curl -s -X POST "$BASE/policies/attachments" -H "Authorization: Bearer $KEY" -H 'Content-Type: application/json' \
  -d '{"policy_name":"repro-lit4199","teams":["this-team-does-not-exist"]}' -w "\n-> %{http_code}\n"

# 2. trailing-* wildcard pattern -> still 200
curl -s -X POST "$BASE/policies/attachments" -H "Authorization: Bearer $KEY" -H 'Content-Type: application/json' \
  -d '{"policy_name":"repro-lit4199","teams":["healthcare-*"]}' -o /dev/null -w "-> %{http_code}\n"

# 3. bogus key and bogus model -> now 400 (covers the sibling fields)
curl -s -X POST "$BASE/policies/attachments" -H "Authorization: Bearer $KEY" -H 'Content-Type: application/json' \
  -d '{"policy_name":"repro-lit4199","keys":["ghost-key"],"models":["ghost-model"]}' -w "\n-> %{http_code}\n"

Output

create policy: 200

1. {"detail":"Team 'this-team-does-not-exist' does not exist. Reference an existing team or use a wildcard pattern (e.g. 'this-team-does-not-exist*') to match by prefix."}
   -> 400

2. -> 200

3. {"detail":"Key 'ghost-key' does not exist. Reference an existing key or use a wildcard pattern (e.g. 'ghost-key*') to match by prefix. Model 'ghost-model' does not exist. Reference an existing model or use a wildcard pattern (e.g. 'ghost-model*') to match by prefix."}
   -> 400

UI, for screenshots:

  1. Open http://localhost:4000/ui/?page=policies and create a policy
  2. Create an attachment from it and choose the Specific scope so the Teams field appears
  3. Type a made-up alias like this-team-does-not-exist, press Enter, then submit; the field now shows an inline error and the attachment is not created
  4. Replace it with a real team alias or a something-* wildcard; submit succeeds
    Uploading Screenshot 2026-07-04 at 11.38.53 AM.png…

Type

Bug Fix

Changes

The policy attachment create endpoint (POST /policies/attachments) accepted whatever teams, keys, and models were passed and persisted them verbatim, so a typo'd or non-existent team was stored silently and the policy applied nowhere. The Admin UI made this easy to hit because the Teams field is an Ant Design Select in tags mode, which accepts arbitrary free text.

create_policy_attachment now validates the scope before writing. It wires in the previously unused PolicyValidator existence checks and rejects any concrete team, key, or model that does not resolve to a real entity, returning a 400 that names the offending value. Whether an entry is "concrete" is decided by RouteChecks._is_wildcard_pattern, the same predicate the request-time matcher uses, so validation and matching agree: only a trailing * is a wildcard. Wildcard patterns are intentionally allowed through since a pattern like healthcare-* may match zero teams today and more once they are created, and tags remain free-form because there is no registry to validate them against.

On the frontend the Teams field keeps free-text entry (wildcards are a feature) but gains a validator that rejects a typed value that is neither an existing team alias nor a wildcard, giving immediate feedback. It only enforces this once the team list has loaded and defers to the backend otherwise, so a failed or partial load never blocks a legitimate team. Keys and models are left to the authoritative backend check rather than pre-validated in the browser, since the form loads only a partial key list and shows model ids rather than the names the backend matches on.

…tachment create

Creating a policy attachment accepted arbitrary team, key, and model values with
no validation, so a typo'd or non-existent team was silently persisted (LIT-4199).
The create endpoint now rejects a concrete (non-wildcard) team, key, or model that
does not resolve to a real entity, wiring the previously-dead PolicyValidator
existence checks and reusing RouteChecks._is_wildcard_pattern so validation agrees
with request-time matching, where only a trailing "*" is a wildcard. Wildcard
patterns are still allowed through since they may match zero entities today and
more later, and tags stay free-form. The Admin UI's Teams field validates the same
rule for immediate feedback when its team list has loaded, deferring to the backend
otherwise.
@greptile-apps

greptile-apps Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR validates scope entries (teams, keys, models) on policy attachment creation, rejecting concrete entries that don't resolve to existing entities and returning a 400 with the offending values named. A matching front-end validator is added for the Teams field in the attachment form.

  • Backend (policy_validator.py, policy_endpoints.py): find_invalid_scope_entries runs async DB existence checks for concrete (non-wildcard) teams and keys via asyncio.gather, plus a sync router check for models. Only trailing * counts as a wildcard, matching RouteChecks._is_wildcard_pattern, so validation and request-time matching are kept in sync.
  • Frontend (scope_validation.ts, add_attachment_form.tsx): getInvalidTeamEntries mirrors the backend predicate; the Teams field gains an antd validator that fires only once the team list has successfully loaded, so a failed fetch defers to the backend rather than blocking valid submissions. Keys and models are left to the authoritative backend check.
  • Tests: Comprehensive unit tests added for both the Python validator and the TypeScript helpers, including wildcard pass-through, fail-open on missing DB, and frontend load-failure deferral.

Confidence Score: 5/5

Safe to merge — the change adds a validation gate that was previously absent, with a clean fail-open posture when the DB or router is unavailable, and comprehensive tests at both layers.

The backend validation is correctly scoped to concrete (non-wildcard) entries, fails open on DB/router errors, and is well-covered by async unit tests using in-process fakes. The frontend validator only activates after a successful team-list load and defers to the backend otherwise. No regressions to existing happy-path behaviour were introduced.

The dead is_wildcard_pattern static method in policy_validator.py (flagged in the previous review thread) is the only item left open; all new files look solid.

Important Files Changed

Filename Overview
litellm/proxy/policy_engine/policy_validator.py Adds find_invalid_scope_entries method that validates concrete scope entries via DB/router; correctly delegates wildcard detection to RouteChecks._is_wildcard_pattern. The existing is_wildcard_pattern static method (which checks for ? or any *) is now dead code whose broader definition conflicts with the new method, but this was already flagged in a previous review thread.
litellm/proxy/policy_engine/policy_endpoints.py Wires PolicyValidator.find_invalid_scope_entries into create_policy_attachment before persistence; raises HTTP 400 with joined error messages on any invalid scope entry. Logic is clean and only runs after confirming DB connectivity.
tests/test_litellm/proxy/policy_engine/test_policy_validator.py New TestAttachmentScopeValidation class with 7 async tests covers: non-existent team/key/model flagged, existing entries pass, trailing-* wildcards allowed, non-trailing * and ? treated as concrete, fail-open when prisma_client=None, and empty scope returns no errors. All tests use in-process fakes — no real network calls.
ui/litellm-dashboard/src/components/policies/add_attachment_form.tsx Adds teamsLoaded flag and antd validator on the Teams field that defers validation until a successful team-list fetch; wildcard entries pass through without a DB check.
ui/litellm-dashboard/src/components/policies/scope_validation.ts New utility: isWildcardPattern (trailing * only) and getInvalidTeamEntries, mirroring backend wildcard semantics. Small and clear.
ui/litellm-dashboard/src/components/policies/scope_validation.test.ts Unit tests for isWildcardPattern and getInvalidTeamEntries; covers wildcard pass-through, concrete alias rejection, and empty input. All pure, no network calls.
ui/litellm-dashboard/src/components/policies/add_attachment_form.test.tsx Four new integration tests cover: non-existent team blocked, existing team passes, wildcard passes, and team-list-load-failure defers to backend. Tests use mocked networking and in-process renders.

Reviews (2): Last reviewed commit: "fix(policies): separate multiple attachm..." | Re-trigger Greptile

Comment thread litellm/proxy/policy_engine/policy_endpoints.py Outdated
@codecov

codecov Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.22222% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
litellm/proxy/policy_engine/policy_endpoints.py 0.00% 5 Missing ⚠️

📢 Thoughts on this report? Let us know!

Keeps the new find_invalid_scope_entries signature off the UP006/UP045 strict
ruff budgets instead of copying the surrounding legacy typing.List/Optional idiom.
Addresses Greptile review: joining per-entry validation messages with a bare
space read as one run-on sentence; ' | ' makes the multi-error 400 detail easier
to parse for users and programmatically.
@ryan-crabbe-berri

Copy link
Copy Markdown
Contributor Author

@greptileai re review

@ryan-crabbe-berri
ryan-crabbe-berri enabled auto-merge (squash) July 4, 2026 18:56
@ryan-crabbe-berri
ryan-crabbe-berri merged commit 23873f8 into litellm_internal_staging Jul 4, 2026
123 of 124 checks passed
@ryan-crabbe-berri
ryan-crabbe-berri deleted the litellm_lit_4199_cannot_select_team_in_attachment branch July 4, 2026 18:58
ryan-crabbe-berri added a commit that referenced this pull request Jul 17, 2026
The policy attachment form fetched /team/list with the caller's own
user_id, which the backend treats as a membership filter even for proxy
admins. Admins only saw teams they were personally a member of, and the
scope validation added in #32131 then rejected every other valid team
alias as nonexistent. Drop the user_id filter; the policies page is
admin-only and /team/list without user_id returns all teams for admin
roles.

Fixes LIT-4199
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