diff --git a/tests/proxy_behavior/management/conftest.py b/tests/proxy_behavior/management/conftest.py index a6da7bd8af4..d69067ae5df 100644 --- a/tests/proxy_behavior/management/conftest.py +++ b/tests/proxy_behavior/management/conftest.py @@ -4,7 +4,7 @@ import os import tempfile import uuid from dataclasses import dataclass -from typing import AsyncIterator +from typing import Any, AsyncIterator, Dict, Optional import httpx import pytest_asyncio @@ -44,10 +44,11 @@ async def proxy_app(): # proxy_startup_event re-reads master_key from LITELLM_MASTER_KEY and # unconditionally overwrites the global, even when initialize() already - # set it from the config YAML. Without these env vars, the entire auth - # stack degrades to user_id=None / non-PROXY_ADMIN for every token. - os.environ.setdefault("LITELLM_MASTER_KEY", MASTER_KEY) - os.environ.setdefault("CONFIG_FILE_PATH", config_path) + # set it from the config YAML. Force (not setdefault) both vars: an + # ambient LITELLM_MASTER_KEY with a different value would make the proxy + # authenticate on that key while the tests still send MASTER_KEY. + os.environ["LITELLM_MASTER_KEY"] = MASTER_KEY + os.environ["CONFIG_FILE_PATH"] = config_path await initialize(config=config_path) @@ -96,6 +97,33 @@ class Scratch: return f"{self.prefix}-{suffix}" if suffix else self.prefix +async def create_scratch_key( + proxy_client, + seeder_cleartext: str, + scratch_prefix: str, + *, + user_id: str, + team_id: Optional[str] = None, + organization_id: Optional[str] = None, +) -> str: + """Seed a scratch-tagged key via /key/generate; returns its cleartext. + + Shared by the write-scenario matrices (key update/regenerate/delete). + """ + body: Dict[str, Any] = {"key_alias": scratch_prefix, "user_id": user_id} + if team_id is not None: + body["team_id"] = team_id + if organization_id is not None: + body["organization_id"] = organization_id + resp = await proxy_client.post( + "/key/generate", + headers={"Authorization": f"Bearer {seeder_cleartext}"}, + json=body, + ) + assert resp.status_code == 200, f"setup failed: {resp.text}" + return resp.json()["key"] + + @pytest_asyncio.fixture async def scratch(prisma): handle = Scratch(prefix=f"{SCRATCH_PREFIX}{uuid.uuid4().hex[:12]}") diff --git a/tests/proxy_behavior/management/regression_replay/README.md b/tests/proxy_behavior/management/regression_replay/README.md deleted file mode 100644 index 008b5a2e4d7..00000000000 --- a/tests/proxy_behavior/management/regression_replay/README.md +++ /dev/null @@ -1,104 +0,0 @@ -# G4 — Regression-replay set for PR1 (Key Tier-1) - -This directory documents the regression-replay verification for the -behavior-pinning suite. For each in-scope recent fix-PR touching -`litellm/proxy/management_endpoints/key_management_endpoints.py`, we record: - -- which scenario(s) in the matrix catch the behavior the fix introduced, -- a RED-at-parent / GREEN-at-tip transcript for at least one canonical PR. - -## Methodology - -For a given fix-PR `` with parent ``: - -```bash -# 1. Save current handler -cp litellm/proxy/management_endpoints/key_management_endpoints.py /tmp/key_mgmt.HEAD.py - -# 2. Replace with the pre-fix version -git show :litellm/proxy/management_endpoints/key_management_endpoints.py \ - > litellm/proxy/management_endpoints/key_management_endpoints.py - -# 3. Run the catching slice — expect RED on the scenarios that flip -DATABASE_URL=postgresql://litellm:litellm@localhost:5432/litellm_test \ - uv run --no-sync pytest tests/proxy_behavior/management/.py -v - -# 4. Restore + confirm GREEN -cp /tmp/key_mgmt.HEAD.py litellm/proxy/management_endpoints/key_management_endpoints.py -DATABASE_URL=postgresql://litellm:litellm@localhost:5432/litellm_test \ - uv run --no-sync pytest tests/proxy_behavior/management/.py -v -``` - -When the fix changed multiple files, the in-place swap is constrained to the -handler module — if a referenced helper moved between modules the swap may -not run; in that case use `git worktree add` for a clean replay. - -## Replay table - -| # | Fix SHA | Subject (truncated) | Endpoint | Catching scenarios | Verified | -|---|---------|---------------------|----------|--------------------|----------| -| 1 | `c7c3df2b02` | extend /key/update admin check to non-budget fields | `/key/update` | `test_key_update.py::self/{team_admin,internal_user,owner,unrelated_same_org,cross_org_user,service_account}` — all 6 flip 200→403 between parent and HEAD | ✅ RED→GREEN below | -| 2 | `8bbc61e03c` | harden /key/update authorization checks | `/key/update` | `test_key_update.py::owner_target/*` — non-admins blocked from updating peers' keys | by-inspection (overlapping coverage with #1) | -| 3 | `1b2756811e` | close project hijacking and key org IDOR | `/key/update` (org_id field) | `test_key_update.py` matrix asserts no row mutation on denied responses (`row.models != [MARKER]`) | by-inspection | -| 4 | `c7c3df2b02` siblings: `f6cd0a827a` | /key/update returns 404 (not 401) for nonexistent body key | `/key/update` | NOT covered by current matrix — the matrix only exercises existing target keys. Future: add 404-on-missing scenarios. | gap (filed below) | -| 5 | `133471f882` | double-counting bug in org/team key limit checks on update | `/key/update` (counting) | NOT directly covered; matrix asserts status only, not counts. Future: budget/limit assertions. | gap (filed below) | -| 6 | `574633fcf1` | exclude budget_limits from deleted verification token | `/key/delete` | `test_key_delete.py` matrix verifies post-delete authentication fails — would catch a shape-of-delete change but not a budget_limits-specific bug | by-inspection (partial) | -| 7 | `db8ef44323` | enforce upperbound_key_generate_params on /key/regenerate | `/key/regenerate` | `test_key_regenerate.py` matrix asserts status; a regen that exceeds upperbound limits would surface IF the test bodies passed disallowed params. Currently they don't. | gap (filed below) | -| 8 | `2220f3076a` | tighten caller-permission checks on key route fields | multiple | spans /key/generate + /key/update; partial overlap with our `team_id`/`user_id` boundary scenarios | by-inspection | -| 9 | `5190bd07eb` | extend caller-permission to service-account | /key/generate, /key/service-account/generate | service_account actor IS in our matrix; some sub-scenarios overlap | by-inspection | -| 10 | `12005c4a02` | /key/aliases auth | `/key/aliases` | not Tier-1 (out of PR1 scope; PR3 territory) | out of scope | -| 11 | `818c097ca9` | Self-exclusion hash mismatch | `/key/update` | partial overlap | by-inspection | -| 12 | `daf7c0c3a8` | virtual keys team filter | `/key/list` | `test_key_list.py` filter param scenarios deferred; default-visibility scenarios already in matrix | gap (deferred) | - -## Verified replay: `c7c3df2b02` - -**Fix subject**: `fix(proxy): extend /key/update admin check to non-budget fields` -**Parent SHA**: `662d05531d` -**Catching slice**: `tests/proxy_behavior/management/test_key_update.py` - -### Parent (pre-fix) — RED - -``` -FAILED tests/proxy_behavior/management/test_key_update.py::test_key_update_authz_matrix[self/team_admin] -FAILED tests/proxy_behavior/management/test_key_update.py::test_key_update_authz_matrix[self/internal_user] -FAILED tests/proxy_behavior/management/test_key_update.py::test_key_update_authz_matrix[self/owner] -FAILED tests/proxy_behavior/management/test_key_update.py::test_key_update_authz_matrix[self/unrelated_same_org] -FAILED tests/proxy_behavior/management/test_key_update.py::test_key_update_authz_matrix[self/cross_org_user] -FAILED tests/proxy_behavior/management/test_key_update.py::test_key_update_authz_matrix[self/service_account] -6 failed, 15 passed -``` - -Each of the 6 self-target scenarios returned `200` under the parent code (the -admin check only gated budget/spend changes, so a non-admin could rewrite -`models`) but is pinned at `403` post-fix. - -### HEAD (post-fix) — GREEN - -``` -21 passed -``` - -The matrix correctly flipped from 6/21 RED to 21/21 GREEN solely on the -handler swap — confirming the suite catches the c7c3df2b02 behavior change. - -## Identified coverage gaps (deferred to PR2/PR3) - -These cells in the replay table call out genuine matrix gaps — the current -PR1 surface does not pin the specific behavior the fix introduced. Each is -worth a follow-up scenario: - -- **404-on-missing-key** (rows 4, paralleling `f6cd0a827a` and - `19efe556cb`) — add an explicit "actor calls /key/update with a body - `key` that does not exist" scenario per endpoint, asserting 404. -- **Budget/limit counting bugs** (row 5, `133471f882`) — add scenarios that - read the team/org spend rows after a denied /key/update and assert no - counter movement. -- **Upperbound enforcement on /key/regenerate** (row 7, `db8ef44323`) — - extend the regenerate matrix with at least one scenario that requests - params exceeding the team's `upperbound_key_generate_params`. -- **`/key/list` filter-param view** (row 12, `daf7c0c3a8`) — PR1 only - pins default-visibility. Filter combinations (`team_id=`, - `include_team_keys=true`, etc.) belong in a follow-up. - -Filed as TODOs rather than blocking PR1: the matrix shape is correct, -these are scope extensions. diff --git a/tests/proxy_behavior/management/test_key_delete.py b/tests/proxy_behavior/management/test_key_delete.py index 455642e9edf..05844ac0031 100644 --- a/tests/proxy_behavior/management/test_key_delete.py +++ b/tests/proxy_behavior/management/test_key_delete.py @@ -1,10 +1,9 @@ -from typing import Any, Dict, Optional - import pytest from litellm.proxy.utils import hash_token from .actors import TEAM_ALPHA, TEAM_BETA, Actor +from .conftest import create_scratch_key pytestmark = pytest.mark.asyncio(loop_scope="session") @@ -36,26 +35,6 @@ _SCENARIOS = [ ] -async def _create_scratch_key( - proxy_client, - seeder_cleartext: str, - scratch_prefix: str, - *, - user_id: str, - team_id: Optional[str] = None, -) -> str: - body: Dict[str, Any] = {"key_alias": scratch_prefix, "user_id": user_id} - if team_id is not None: - body["team_id"] = team_id - resp = await proxy_client.post( - "/key/generate", - headers={"Authorization": f"Bearer {seeder_cleartext}"}, - json=body, - ) - assert resp.status_code == 200, f"setup failed: {resp.text}" - return resp.json()["key"] - - @pytest.mark.parametrize( "actor,target_shape,expected_status", [(a, t, s) for (_id, a, t, s) in _SCENARIOS], @@ -74,11 +53,11 @@ async def test_key_delete_authz_matrix( seeder = world.keys[Actor.PROXY_ADMIN].cleartext if target_shape == "self": - target_cleartext = await _create_scratch_key( + target_cleartext = await create_scratch_key( proxy_client, seeder, scratch.prefix, user_id=caller.user_id ) elif target_shape == "owner": - target_cleartext = await _create_scratch_key( + target_cleartext = await create_scratch_key( proxy_client, seeder, scratch.prefix, @@ -86,7 +65,7 @@ async def test_key_delete_authz_matrix( team_id=TEAM_ALPHA, ) elif target_shape == "cross_org": - target_cleartext = await _create_scratch_key( + target_cleartext = await create_scratch_key( proxy_client, seeder, scratch.prefix, diff --git a/tests/proxy_behavior/management/test_key_list.py b/tests/proxy_behavior/management/test_key_list.py index bfcd97c66a6..bda8788c9a7 100644 --- a/tests/proxy_behavior/management/test_key_list.py +++ b/tests/proxy_behavior/management/test_key_list.py @@ -21,6 +21,27 @@ _VISIBILITY = { } +async def _all_visible_hashes(proxy_client, caller_cleartext) -> set: + """Walk every /key/list page — size is capped at 100 by the endpoint, so a + single request can truncate PROXY_ADMIN's view on a non-fresh DB.""" + hashes: set = set() + page = 1 + while True: + resp = await proxy_client.get( + f"/key/list?page={page}&size=100", + headers={"Authorization": f"Bearer {caller_cleartext}"}, + ) + assert resp.status_code == 200, resp.text + body = resp.json() + for entry in body.get("keys", []): + tok = entry.get("token") if isinstance(entry, dict) else entry + if tok: + hashes.add(tok) + if page >= (body.get("total_pages") or 1): + return hashes + page += 1 + + @pytest.mark.parametrize( "actor,expected_visible", list(_VISIBILITY.items()), @@ -32,16 +53,7 @@ async def test_key_list_visibility( caller = world.keys[actor] hashed_to_actor = {world.keys[a].hashed: a for a in Actor} - resp = await proxy_client.get( - "/key/list?size=100", - headers={"Authorization": f"Bearer {caller.cleartext}"}, - ) - assert resp.status_code == 200, f"{actor.value}: {resp.text}" - - returned_hashes = { - (entry.get("token") if isinstance(entry, dict) else entry) - for entry in resp.json().get("keys", []) - } + returned_hashes = await _all_visible_hashes(proxy_client, caller.cleartext) visible_seeded = { hashed_to_actor[h] for h in returned_hashes if h in hashed_to_actor } diff --git a/tests/proxy_behavior/management/test_key_regenerate.py b/tests/proxy_behavior/management/test_key_regenerate.py index 906dcc9c7be..a3289144eef 100644 --- a/tests/proxy_behavior/management/test_key_regenerate.py +++ b/tests/proxy_behavior/management/test_key_regenerate.py @@ -1,8 +1,7 @@ -from typing import Any, Dict, Optional - import pytest from .actors import TEAM_ALPHA, TEAM_BETA, Actor +from .conftest import create_scratch_key pytestmark = pytest.mark.asyncio(loop_scope="session") @@ -35,26 +34,6 @@ _SCENARIOS = [ ] -async def _create_scratch_key( - proxy_client, - seeder_cleartext: str, - scratch_prefix: str, - *, - user_id: str, - team_id: Optional[str] = None, -) -> str: - body: Dict[str, Any] = {"key_alias": scratch_prefix, "user_id": user_id} - if team_id is not None: - body["team_id"] = team_id - resp = await proxy_client.post( - "/key/generate", - headers={"Authorization": f"Bearer {seeder_cleartext}"}, - json=body, - ) - assert resp.status_code == 200, f"setup failed: {resp.text}" - return resp.json()["key"] - - async def _info(proxy_client, cleartext: str): return await proxy_client.get( "/key/info", headers={"Authorization": f"Bearer {cleartext}"} @@ -78,11 +57,11 @@ async def test_key_regenerate_authz_matrix( seeder = world.keys[Actor.PROXY_ADMIN].cleartext if target_shape == "self": - target_cleartext = await _create_scratch_key( + target_cleartext = await create_scratch_key( proxy_client, seeder, scratch.prefix, user_id=caller.user_id ) elif target_shape == "owner": - target_cleartext = await _create_scratch_key( + target_cleartext = await create_scratch_key( proxy_client, seeder, scratch.prefix, @@ -90,7 +69,7 @@ async def test_key_regenerate_authz_matrix( team_id=TEAM_ALPHA, ) elif target_shape == "cross_org": - target_cleartext = await _create_scratch_key( + target_cleartext = await create_scratch_key( proxy_client, seeder, scratch.prefix, @@ -122,7 +101,7 @@ async def test_key_regenerate_authz_matrix( async def test_key_path_regenerate_smoke(proxy_client, scratch, world): """Pins that POST /key/{key:path}/regenerate shares the same handler.""" caller = world.keys[Actor.PROXY_ADMIN] - target_cleartext = await _create_scratch_key( + target_cleartext = await create_scratch_key( proxy_client, caller.cleartext, scratch.prefix, user_id=caller.user_id ) diff --git a/tests/proxy_behavior/management/test_key_update.py b/tests/proxy_behavior/management/test_key_update.py index d0dc401feef..36ddefa5750 100644 --- a/tests/proxy_behavior/management/test_key_update.py +++ b/tests/proxy_behavior/management/test_key_update.py @@ -1,10 +1,9 @@ -from typing import Any, Dict, Optional - import pytest from litellm.proxy.utils import hash_token from .actors import TEAM_ALPHA, TEAM_BETA, Actor +from .conftest import create_scratch_key pytestmark = pytest.mark.asyncio(loop_scope="session") @@ -40,29 +39,6 @@ _SCENARIOS = [ MARKER_MODEL = "behavior-pin-update-marker-model" -async def _create_scratch_key( - proxy_client, - seeder_cleartext: str, - scratch_prefix: str, - *, - user_id: str, - team_id: Optional[str] = None, - organization_id: Optional[str] = None, -) -> str: - body: Dict[str, Any] = {"key_alias": scratch_prefix, "user_id": user_id} - if team_id is not None: - body["team_id"] = team_id - if organization_id is not None: - body["organization_id"] = organization_id - resp = await proxy_client.post( - "/key/generate", - headers={"Authorization": f"Bearer {seeder_cleartext}"}, - json=body, - ) - assert resp.status_code == 200, f"setup failed: {resp.text}" - return resp.json()["key"] - - @pytest.mark.parametrize( "actor,target_shape,expected_status", [(a, t, s) for (_id, a, t, s) in _SCENARIOS], @@ -81,11 +57,11 @@ async def test_key_update_authz_matrix( seeder = world.keys[Actor.PROXY_ADMIN].cleartext if target_shape == "self": - target_cleartext = await _create_scratch_key( + target_cleartext = await create_scratch_key( proxy_client, seeder, scratch.prefix, user_id=caller.user_id ) elif target_shape == "owner": - target_cleartext = await _create_scratch_key( + target_cleartext = await create_scratch_key( proxy_client, seeder, scratch.prefix, @@ -93,7 +69,7 @@ async def test_key_update_authz_matrix( team_id=TEAM_ALPHA, ) elif target_shape == "cross_org": - target_cleartext = await _create_scratch_key( + target_cleartext = await create_scratch_key( proxy_client, seeder, scratch.prefix,