diff --git a/tests/proxy_behavior/management/README.md b/tests/proxy_behavior/management/README.md deleted file mode 100644 index 0c2f9cb99d3..00000000000 --- a/tests/proxy_behavior/management/README.md +++ /dev/null @@ -1,107 +0,0 @@ -# Management-endpoint behavior-pinning suite - -HTTP-boundary regression tests for -`litellm/proxy/management_endpoints/key_management_endpoints.py` (PR1 — Key -Tier-1). Runs against the real proxy app via in-process `httpx.ASGITransport`, -connected to a real Postgres pointed at by `DATABASE_URL`. **No mocks** — -auth runs, prisma runs, integrations run. Test bodies make HTTP calls and -assert at the API boundary. - -The eventual goal (across PR1–PR3) is to pin every authorization / -cross-tenant / budget-bypass boundary on the key + team management -surfaces. PR1 covers six Tier-1 key endpoints (`/key/generate`, `/key/info`, -`/key/list`, `/key/update`, `/key/regenerate`, `/key/delete`). See the -[Notion plan](https://www.notion.so/36643b8acdab8128a581ced0f6a4744d) for -the full scope. - -## Local repro - -Identical to the three commands the CI workflow -(`.github/workflows/test-unit-proxy-mgmt-behavior.yml`, which delegates -to `_test-unit-services-base.yml`) runs: - -```bash -# 1. Bring up Postgres -docker run --rm -d --name litellm-test-pg \ - -e POSTGRES_USER=litellm -e POSTGRES_PASSWORD=litellm -e POSTGRES_DB=litellm_test \ - -p 5432:5432 postgres:14 - -# 2. Migrate the schema (one-time per fresh DB) -export DATABASE_URL=postgresql://litellm:litellm@localhost:5432/litellm_test -uv run prisma generate --schema litellm/proxy/schema.prisma -uv run prisma db push --schema litellm/proxy/schema.prisma --accept-data-loss - -# 3. Run the suite -uv run pytest tests/proxy_behavior/management/ -``` - -Whole-suite wall-time is ~6s on a warm cache (one ~1.5s session setup + -~0.01–0.04s per test). Re-running back-to-back produces identical pass -counts — the scratch-namespace teardown leaves no rows behind. - -### Single scenario / inner loop - -```bash -uv run pytest tests/proxy_behavior/management/test_key_update.py -k self/owner -v -``` - -## Layout - -``` -tests/proxy_behavior/management/ -├── conftest.py # session ASGI client, world seed, scratch fixture -├── actors.py # 8-actor enum + seed_world() helper -├── test_smoke.py # liveness + key/generate de-risk smoke -├── test_world_seed.py # every seeded actor key authenticates -├── test_scratch_teardown.py # scratch namespace cleanup invariants -├── test_no_management_imports.py # G3 — strict-import grep as a test -├── test_key_generate.py # Slice 7 — actor × target matrix -├── test_key_info.py # Slice 8 -├── test_key_list.py # Slice 9 -├── test_key_update.py # Slice 10 -├── test_key_regenerate.py # Slice 11 -├── test_key_delete.py # Slice 12 -└── regression_replay/README.md # G4 — fix-PR → catching-scenario mapping -``` - -## Conventions - -- **Async fixture / loop scope.** `pyproject.toml` sets - `asyncio_default_fixture_loop_scope = "session"`, but the default - *test* loop scope is per-function. Add - `pytestmark = pytest.mark.asyncio(loop_scope="session")` at the top - of every test file so the AsyncClient and prisma connection (both - session-scoped) share a loop with the test body. -- **Forbidden imports (G3).** No `from litellm.proxy.management_endpoints`, - no `mock`/`patch` on `user_api_key_auth`. Enforced by - `test_no_management_imports.py` as a pytest item. -- **Read-world vs scratch-world.** The `world` fixture seeds an immutable - read-world under the `behavior-pin-` prefix; tests must not mutate - those rows. The `scratch` fixture gives a per-test - `scratch-` prefix and tears down any row tagged with it. - Write scenarios always tag their creates with `scratch.prefix`. -- **Behavior pinning, not behavior judging.** Expected status codes are - pinned against current handler behavior. The suite's job is to make - *changes* to that behavior visible — not to assert what the codes - *should* be. Comments above each `_SCENARIOS` block call out - surprising or potentially-buggy behaviors for human review. - -## Gate evidence - -PR1's evidence for each G1–G5 + PR1.M1–M3 gate lives in: - -- **G1** — CI run on the PR's workflow `test-unit-proxy-mgmt-behavior` (green). -- **G2** — `pytest --durations=…` summary in the PR description (≤ 10 min). -- **G3** — `test_no_management_imports.py` is part of the suite itself. -- **G4** — `regression_replay/README.md`. -- **G5** — Trigger `.github/workflows/mutation-test.yml` manually - (`gh workflow run mutation-test.yml`); the workflow uses the - `[tool.mutmut].tests_dir` entries in `pyproject.toml` to include this - suite. Triage every surviving mutant inside the six Tier-1 handler - functions (kill or accept-with-reason) — zero unreviewed survivors is - the binding gate. Record the survivor count + kill rate in a follow-up - PR comment or a triage doc filed alongside that follow-up (we don't - commit empty stubs). -- **PR1.M1** — total scenario count, this README's "Layout" section. -- **PR1.M2** — this README + the workflow YAML are the local-repro contract. -- **PR1.M3** — Recorded in the manual mutmut follow-up (see G5 above). diff --git a/tests/proxy_behavior/management/actors.py b/tests/proxy_behavior/management/actors.py index f3982c18e85..1bcf8ed474d 100644 --- a/tests/proxy_behavior/management/actors.py +++ b/tests/proxy_behavior/management/actors.py @@ -1,13 +1,4 @@ -"""Immutable read-world seed for behavior-pinning tests. - -Seeds 8 actor profiles across 2 orgs / 2 teams so authz matrix tests can drive -``(actor, target, expected)`` tuples against the real proxy. Each actor has -exactly one virtual key (cleartext + hashed) so tests pass the cleartext as a -``Bearer`` token and the real auth stack accepts it. - -All seeded rows are namespaced under the ``behavior-pin-`` prefix on their -primary key, so the seed is identifiable in psql and the wipe step is targeted. -""" +"""8-actor read-world seed for the authz matrix tests.""" import enum import uuid @@ -31,7 +22,6 @@ class Actor(str, enum.Enum): SERVICE_ACCOUNT = "service_account" -# Stable IDs so re-seed is idempotent and the world is identifiable in psql. PREFIX = "behavior-pin-" ORG_A = PREFIX + "org-a" ORG_B = PREFIX + "org-b" @@ -61,19 +51,6 @@ def _new_clear_key() -> str: def _actor_profile() -> Dict[Actor, Dict[str, Any]]: - """Per-actor (role, scoping) profile used for both the user row and its key. - - Scoping rules: - - PROXY_ADMIN: global, no team/org scope on its key. - - ORG_ADMIN: org_a, no team scope. - - TEAM_ADMIN / INTERNAL_USER / OWNER / UNRELATED_SAME_ORG / SERVICE_ACCOUNT: - team_alpha within org_a. - - CROSS_ORG_USER: team_beta within org_b. - - The auth layer reads ``user_role`` off the user row pointed at by the key's - ``user_id``, so setting it once on the user is enough — keys do not need a - separate role field. - """ return { Actor.PROXY_ADMIN: { "user_role": LitellmUserRoles.PROXY_ADMIN.value, @@ -119,7 +96,6 @@ def _actor_profile() -> Dict[Actor, Dict[str, Any]]: async def _wipe_world(prisma: PrismaClient) -> None: - """Delete prior seed rows so re-seed is idempotent across sessions.""" await prisma.db.litellm_verificationtoken.delete_many( where={"user_id": {"startswith": PREFIX}} ) @@ -144,7 +120,6 @@ async def _wipe_world(prisma: PrismaClient) -> None: async def seed_world(prisma: PrismaClient) -> World: await _wipe_world(prisma) - # Budget — orgs require a non-null budget_id. await prisma.db.litellm_budgettable.create( data={ "budget_id": BUDGET_ID, @@ -153,7 +128,6 @@ async def seed_world(prisma: PrismaClient) -> World: } ) - # Orgs. for org_id, alias in [(ORG_A, "alpha"), (ORG_B, "beta")]: await prisma.db.litellm_organizationtable.create( data={ @@ -168,7 +142,6 @@ async def seed_world(prisma: PrismaClient) -> World: profiles = _actor_profile() user_ids: Dict[Actor, str] = {actor: PREFIX + actor.value for actor in Actor} - # Users. for actor, profile in profiles.items(): teams_list = [profile["team_id"]] if profile["team_id"] else [] await prisma.db.litellm_usertable.create( @@ -181,12 +154,9 @@ async def seed_world(prisma: PrismaClient) -> World: } ) - # Teams. - # NOTE: _get_user_in_team in key_management_endpoints.py checks - # ``members_with_roles`` (a JSON array of {user_id, role}), NOT the plain - # ``members`` String[] column — so the JSON list is what the team-key authz - # gate inspects. Populate both to match what the real /team/new handler - # would produce. + # _get_user_in_team in key_management_endpoints.py walks members_with_roles + # (a JSON list of {user_id, role}), not the String[] members column — + # populate both to match what /team/new produces. await prisma.db.litellm_teamtable.create( data={ "team_id": TEAM_ALPHA, @@ -226,7 +196,6 @@ async def seed_world(prisma: PrismaClient) -> World: } ) - # Org memberships (UI lookups + ORG_ADMIN scoping use these). for actor, org_id, role in [ (Actor.ORG_ADMIN, ORG_A, "org_admin"), (Actor.TEAM_ADMIN, ORG_A, "internal_user"), @@ -244,7 +213,6 @@ async def seed_world(prisma: PrismaClient) -> World: } ) - # Team memberships. for actor, team_id in [ (Actor.TEAM_ADMIN, TEAM_ALPHA), (Actor.INTERNAL_USER, TEAM_ALPHA), @@ -257,8 +225,6 @@ async def seed_world(prisma: PrismaClient) -> World: data={"user_id": user_ids[actor], "team_id": team_id} ) - # Verification tokens — one cleartext per actor, hashed via the real - # credential boundary so user_api_key_auth accepts it. keys: Dict[Actor, SeededKey] = {} for actor, profile in profiles.items(): cleartext = _new_clear_key() @@ -267,8 +233,8 @@ async def seed_world(prisma: PrismaClient) -> World: "token": hashed, "key_name": PREFIX + actor.value + "-key", "user_id": user_ids[actor], - # LiteLLM_VerificationTokenView is non-Optional on models — Postgres lets the - # column be NULL, but the pydantic view used by user_api_key_auth rejects None. + # LiteLLM_VerificationTokenView's models field rejects NULL even + # though the column is nullable in Postgres. "models": [], } if profile["team_id"]: @@ -276,7 +242,6 @@ async def seed_world(prisma: PrismaClient) -> World: if profile["organization_id"]: token_data["organization_id"] = profile["organization_id"] if actor == Actor.SERVICE_ACCOUNT: - # LiteLLM convention: service-account keys carry an explicit metadata flag. token_data["metadata"] = Json({"service_account_id": user_ids[actor]}) await prisma.db.litellm_verificationtoken.create(data=token_data) keys[actor] = SeededKey( diff --git a/tests/proxy_behavior/management/conftest.py b/tests/proxy_behavior/management/conftest.py index 8e37a460e96..a6da7bd8af4 100644 --- a/tests/proxy_behavior/management/conftest.py +++ b/tests/proxy_behavior/management/conftest.py @@ -1,9 +1,4 @@ -"""Session-scoped async ASGI client for behavior-pinning tests. - -The proxy app is initialised once per pytest session against the real Postgres -pointed at by ``DATABASE_URL``. No mocks: auth runs, prisma runs, integrations -run. Tests assert at the HTTP boundary. -""" +"""Session-scoped async ASGI client for HTTP-boundary behavior tests.""" import os import tempfile @@ -17,6 +12,7 @@ import yaml MASTER_KEY = "sk-1234" +SCRATCH_PREFIX = "scratch-" def _write_minimal_proxy_config() -> str: @@ -27,7 +23,6 @@ def _write_minimal_proxy_config() -> str: database_url = os.environ.get("DATABASE_URL") if database_url: config["general_settings"]["database_url"] = database_url - f = tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) yaml.dump(config, f) f.close() @@ -36,13 +31,7 @@ def _write_minimal_proxy_config() -> str: @pytest_asyncio.fixture(scope="session") async def proxy_app(): - """Boot the proxy app once per session with the real FastAPI lifespan. - - httpx 0.28's ASGITransport does not run the lifespan handler, so we enter - ``proxy_startup_event`` (the @asynccontextmanager registered as the app's - lifespan) directly. That handler is where ``prisma_client`` is connected - and the rest of the startup wiring runs. - """ + from litellm.proxy import proxy_server from litellm.proxy.proxy_server import ( app, cleanup_router_config_variables, @@ -53,45 +42,25 @@ async def proxy_app(): cleanup_router_config_variables() config_path = _write_minimal_proxy_config() - # proxy_startup_event re-reads master_key from LITELLM_MASTER_KEY (line 776 - # in proxy_server.py). If unset, the global master_key is overwritten to - # None *after* initialize()'s config-derived value, and the entire auth - # stack falls into a degraded path that produces user_id=None and a - # non-PROXY_ADMIN role for every key, including the master key itself. - # Locally this is masked when LITELLM_MASTER_KEY happens to be set in the - # shell; CI is clean, which is how this surfaced. + # 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) await initialize(config=config_path) - # /key/regenerate (and a few other Tier-1 endpoints) are gated behind - # ``premium_user`` — without a LITELLM_LICENSE the proxy returns 500 - # "Enterprise feature" for those calls. The behavior matrix isn't about - # licensing; it's about authz. Force the proxy into premium mode so the - # matrix pins the real authz behavior, not the licensing gate. - from litellm.proxy import proxy_server as _proxy_server - - _proxy_server.premium_user = True + # /key/regenerate is gated behind premium_user; flipping it lets the matrix + # pin authz behavior instead of the licensing gate. + proxy_server.premium_user = True async with proxy_startup_event(app): - # The lifespan re-runs ``premium_user = _license_check.is_premium()`` - # which flips it back. Force it again after the lifespan settles. - _proxy_server.premium_user = True - - # The lifespan kicks off ``prisma_client.check_view_exists()`` as a - # fire-and-forget background task. That task creates the - # ``LiteLLM_VerificationTokenView`` SQL view used by ``user_api_key_auth`` - # to resolve a token to its user / role / team. On a fresh Postgres - # (CI), the first test races the task — the view doesn't exist yet, - # ``user_api_key_dict.user_id`` resolves to ``None``, and every authz - # check that depends on it fails confusingly. Locally the view already - # exists from prior runs, masking the race. Await it explicitly here - # so the suite is deterministic regardless of DB state. - from litellm.proxy import proxy_server as _proxy_server - - if _proxy_server.prisma_client is not None: - await _proxy_server.prisma_client.check_view_exists() + proxy_server.premium_user = True # lifespan re-runs _license_check + # The lifespan fires check_view_exists() as a background task; on a + # fresh DB the first auth call races it and resolves user_id=None. + if proxy_server.prisma_client is not None: + await proxy_server.prisma_client.check_view_exists() yield app @@ -106,41 +75,21 @@ async def proxy_client(proxy_app) -> AsyncIterator[httpx.AsyncClient]: @pytest_asyncio.fixture(scope="session") async def prisma(proxy_app): - """The connected PrismaClient the lifespan opened.""" from litellm.proxy import proxy_server - assert ( - proxy_server.prisma_client is not None - ), "FastAPI lifespan did not connect prisma — harness is wrong." + assert proxy_server.prisma_client is not None return proxy_server.prisma_client @pytest_asyncio.fixture(scope="session") async def world(prisma): - """The immutable read-world seed. - - Re-seeds at session start so each pytest invocation gets a clean world. - Tests must not mutate these rows; write tests use the ``scratch`` fixture - below for scoped entities that get torn down per-test. - """ from .actors import seed_world return await seed_world(prisma) -SCRATCH_PREFIX = "scratch-" - - @dataclass(frozen=True) class Scratch: - """Per-test namespace for write scenarios. - - Tests must tag any entity they create with ``scratch.prefix`` in a column - the teardown filter inspects (``key_alias``, ``key_name``, ``team_alias``, - ``team_id``, ``user_id``, or ``budget_id``). Anything not tagged will be - left behind and pollute the next session. - """ - prefix: str def tag(self, suffix: str = "") -> str: @@ -149,17 +98,11 @@ class Scratch: @pytest_asyncio.fixture async def scratch(prisma): - """Function-scoped scratch namespace + targeted delete_many teardown. - - The teardown deletes any rows on the volatile tables whose namespace column - starts with ``scratch.prefix``. Per CLAUDE.md, this is Prisma-only — no raw - SQL — and uses ``delete_many`` to batch the writes. - """ handle = Scratch(prefix=f"{SCRATCH_PREFIX}{uuid.uuid4().hex[:12]}") try: yield handle finally: - # Order matters: children before parents to avoid FK conflicts. + # Children before parents to avoid FK violations. await prisma.db.litellm_verificationtoken.delete_many( where={ "OR": [ diff --git a/tests/proxy_behavior/management/test_key_delete.py b/tests/proxy_behavior/management/test_key_delete.py index 51668e37830..455642e9edf 100644 --- a/tests/proxy_behavior/management/test_key_delete.py +++ b/tests/proxy_behavior/management/test_key_delete.py @@ -1,22 +1,17 @@ -"""Slice 12 — actor × target authz matrix for ``POST /key/delete``. - -Same shape as Slices 10/11: master-seed a scoped scratch key, the actor under -test attempts to delete it via ``POST /key/delete {keys: []}``. On -200 the test verifies the row is gone (or soft-deleted) AND the cleartext can -no longer auth. On denial it verifies the row survives and still authenticates. -""" - from typing import Any, Dict, Optional import pytest +from litellm.proxy.utils import hash_token + from .actors import TEAM_ALPHA, TEAM_BETA, Actor pytestmark = pytest.mark.asyncio(loop_scope="session") +# Same-team peers can READ each other's keys (see test_key_info) but cannot +# DELETE them — delete is stricter than read. _SCENARIOS = [ - # ─── target = self-owned key ────────────────────────────────────────── ("self/proxy_admin", Actor.PROXY_ADMIN, "self", 200), ("self/org_admin", Actor.ORG_ADMIN, "self", 401), ("self/team_admin", Actor.TEAM_ADMIN, "self", 200), @@ -25,16 +20,13 @@ _SCENARIOS = [ ("self/unrelated_same_org", Actor.UNRELATED_SAME_ORG, "self", 200), ("self/cross_org_user", Actor.CROSS_ORG_USER, "self", 200), ("self/service_account", Actor.SERVICE_ACCOUNT, "self", 200), - # ─── target = OWNER-scoped key in org_a / team_alpha ────────────────── ("owner_target/proxy_admin", Actor.PROXY_ADMIN, "owner", 200), - # ORG_ADMIN hits the early role gate before any target-specific check. ("owner_target/org_admin", Actor.ORG_ADMIN, "owner", 401), ("owner_target/team_admin", Actor.TEAM_ADMIN, "owner", 200), ("owner_target/internal_user", Actor.INTERNAL_USER, "owner", 403), ("owner_target/unrelated_same_org", Actor.UNRELATED_SAME_ORG, "owner", 403), ("owner_target/cross_org_user", Actor.CROSS_ORG_USER, "owner", 403), ("owner_target/service_account", Actor.SERVICE_ACCOUNT, "owner", 403), - # ─── target = CROSS_ORG_USER-scoped key in org_b / team_beta ────────── ("cross_org_target/proxy_admin", Actor.PROXY_ADMIN, "cross_org", 200), ("cross_org_target/org_admin", Actor.ORG_ADMIN, "cross_org", 401), ("cross_org_target/team_admin", Actor.TEAM_ADMIN, "cross_org", 403), @@ -52,14 +44,6 @@ async def _create_scratch_key( user_id: str, team_id: Optional[str] = None, ) -> str: - """Seed a scratch key using the proxy_admin actor (not the bare master key). - - The seeded proxy_admin actor's auth path produces user_role=PROXY_ADMIN - + a concrete user_id from the DB, which deterministically triggers the - ``_user_can_only_create_keys_for_themselves`` PROXY_ADMIN bypass. The - bare master key takes a different auth resolution path whose behavior - differs between fresh-CI and warm-local environments. - """ body: Dict[str, Any] = {"key_alias": scratch_prefix, "user_id": user_id} if team_id is not None: body["team_id"] = team_id @@ -68,7 +52,7 @@ async def _create_scratch_key( headers={"Authorization": f"Bearer {seeder_cleartext}"}, json=body, ) - assert resp.status_code == 200, f"setup: seeder /key/generate failed: {resp.text}" + assert resp.status_code == 200, f"setup failed: {resp.text}" return resp.json()["key"] @@ -86,21 +70,17 @@ async def test_key_delete_authz_matrix( scratch, world, ): - from litellm.proxy.utils import hash_token - caller = world.keys[actor] + seeder = world.keys[Actor.PROXY_ADMIN].cleartext if target_shape == "self": target_cleartext = await _create_scratch_key( - proxy_client, - world.keys[Actor.PROXY_ADMIN].cleartext, - scratch.prefix, - user_id=caller.user_id, + proxy_client, seeder, scratch.prefix, user_id=caller.user_id ) elif target_shape == "owner": target_cleartext = await _create_scratch_key( proxy_client, - world.keys[Actor.PROXY_ADMIN].cleartext, + seeder, scratch.prefix, user_id=world.keys[Actor.OWNER].user_id, team_id=TEAM_ALPHA, @@ -108,7 +88,7 @@ async def test_key_delete_authz_matrix( elif target_shape == "cross_org": target_cleartext = await _create_scratch_key( proxy_client, - world.keys[Actor.PROXY_ADMIN].cleartext, + seeder, scratch.prefix, user_id=world.keys[Actor.CROSS_ORG_USER].user_id, team_id=TEAM_BETA, @@ -123,34 +103,20 @@ async def test_key_delete_authz_matrix( headers={"Authorization": f"Bearer {caller.cleartext}"}, json={"keys": [target_cleartext]}, ) - assert resp.status_code == expected_status, ( - f"{actor.value} POST /key/delete {target_shape} → " - f"{resp.status_code} (expected {expected_status}). body={resp.text}" - ) + assert ( + resp.status_code == expected_status + ), f"{actor.value} {target_shape}: {resp.status_code} {resp.text}" - # Verify the after-state matches the verdict. row = await prisma.db.litellm_verificationtoken.find_unique( where={"token": target_hashed} ) auth_check = await proxy_client.get( - "/key/info", - headers={"Authorization": f"Bearer {target_cleartext}"}, + "/key/info", headers={"Authorization": f"Bearer {target_cleartext}"} ) if expected_status == 200: - # Successful delete: cleartext must no longer authenticate, regardless of - # whether the row is hard-deleted or soft-deleted into LiteLLM_DeletedVerificationToken. - assert auth_check.status_code == 401, ( - f"{actor.value}: handler returned 200 but cleartext still authenticates " - f"({auth_check.status_code}): {auth_check.text}" - ) + # Hard- or soft-delete both produce a 401 on subsequent auth. + assert auth_check.status_code == 401 else: - # Denied: row still present, cleartext still works. - assert row is not None, ( - f"{actor.value}: handler returned {expected_status} but row vanished — " - f"silent delete on denial" - ) - assert auth_check.status_code == 200, ( - f"{actor.value}: handler returned {expected_status} but cleartext no " - f"longer authenticates: {auth_check.text}" - ) + assert row is not None, f"{actor.value}: denied but row vanished" + assert auth_check.status_code == 200 diff --git a/tests/proxy_behavior/management/test_key_generate.py b/tests/proxy_behavior/management/test_key_generate.py index 22cf681136d..851de33d3ff 100644 --- a/tests/proxy_behavior/management/test_key_generate.py +++ b/tests/proxy_behavior/management/test_key_generate.py @@ -1,39 +1,18 @@ -"""Slice 7 — actor × target authz matrix for ``POST /key/generate``. - -The matrix pins the proxy's current authorization behavior for key creation. -Two boundary axes: - - * **Self-create.** Each of the 8 seeded actors creates a key with no - ``team_id`` / no ``user_id`` override. The expected outcome is the actor's - current right to create a virtual key for themselves. - * **Cross-scope create.** A subset of actors create a key scoped to a team - that may or may not match their org / team membership. This is the IDOR - boundary — a passing test means an unauthorized actor was *correctly* - blocked; a failing test (after a refactor) means the boundary moved. - -Expected status codes were observed against the real handler and pinned here. -Future PRs that change these codes will turn this matrix red, surfacing the -behavior change for review. -""" - -from typing import Any, Dict, Optional +from typing import Any, Dict import pytest from .actors import TEAM_ALPHA, TEAM_BETA, Actor -from .conftest import MASTER_KEY pytestmark = pytest.mark.asyncio(loop_scope="session") -# Each row is (id, actor, body_extras, expected_status). Codes are PINNED -# against the current handler's observed behavior — the point of these tests -# is to red-flag *changes* to that behavior, not to assert what's ideal. -# A future PR that flips any code here is a behavior change that needs review. +# (id, actor, body_extras, expected_status). Status codes pinned to observed +# handler behavior — heterogeneous (200, 400, 401) because the handler routes +# denials through three different gates (role gate, user_id mismatch, team +# member permission). _SCENARIOS = [ - # ─── Self-create: actor generates a key for themselves ─────────────── ("self/proxy_admin", Actor.PROXY_ADMIN, {}, 200), - # org_admin currently 401s on /key/generate (role-gate before scope check). ("self/org_admin", Actor.ORG_ADMIN, {}, 401), ("self/team_admin", Actor.TEAM_ADMIN, {}, 200), ("self/internal_user", Actor.INTERNAL_USER, {}, 200), @@ -41,23 +20,15 @@ _SCENARIOS = [ ("self/unrelated_same_org", Actor.UNRELATED_SAME_ORG, {}, 200), ("self/cross_org_user", Actor.CROSS_ORG_USER, {}, 200), ("self/service_account", Actor.SERVICE_ACCOUNT, {}, 200), - # ─── team_id = team_alpha (org_a) ───────────────────────────────────── ("team_alpha/proxy_admin", Actor.PROXY_ADMIN, {"team_id": TEAM_ALPHA}, 200), - # org_admin is blocked by the role gate before the team scope check runs. ("team_alpha/org_admin", Actor.ORG_ADMIN, {"team_id": TEAM_ALPHA}, 401), - # team_admin is admin of team_alpha — allowed. ("team_alpha/team_admin", Actor.TEAM_ADMIN, {"team_id": TEAM_ALPHA}, 200), - # Regular team member without key-create permissions: 401 + team_member_permission_error. ("team_alpha/internal_user", Actor.INTERNAL_USER, {"team_id": TEAM_ALPHA}, 401), - # Cross-org user is "not assigned" — 400 fires before team-member-perms. ("team_alpha/cross_org_user", Actor.CROSS_ORG_USER, {"team_id": TEAM_ALPHA}, 400), - # ─── team_id = team_beta (org_b) ────────────────────────────────────── ("team_beta/proxy_admin", Actor.PROXY_ADMIN, {"team_id": TEAM_BETA}, 200), ("team_beta/org_admin", Actor.ORG_ADMIN, {"team_id": TEAM_BETA}, 401), - # team_admin is not a member of team_beta → "not assigned" 400. ("team_beta/team_admin", Actor.TEAM_ADMIN, {"team_id": TEAM_BETA}, 400), ("team_beta/internal_user", Actor.INTERNAL_USER, {"team_id": TEAM_BETA}, 400), - # cross_org_user IS a member of team_beta (no admin) → team_member_perm 401. ("team_beta/cross_org_user", Actor.CROSS_ORG_USER, {"team_id": TEAM_BETA}, 401), ] @@ -65,7 +36,7 @@ _SCENARIOS = [ @pytest.mark.parametrize( "actor,body_extras,expected_status", [(actor, body, expected) for (_id, actor, body, expected) in _SCENARIOS], - ids=[scenario[0] for scenario in _SCENARIOS], + ids=[s[0] for s in _SCENARIOS], ) async def test_key_generate_authz_matrix( actor: Actor, @@ -84,28 +55,16 @@ async def test_key_generate_authz_matrix( headers={"Authorization": f"Bearer {seeded.cleartext}"}, json=body, ) - assert resp.status_code == expected_status, ( - f"{actor.value} POST /key/generate {body!r} → {resp.status_code} " - f"(expected {expected_status}). body={resp.text}" - ) + assert ( + resp.status_code == expected_status + ), f"{actor.value} {body!r} → {resp.status_code}: {resp.text}" + rows = await prisma.db.litellm_verificationtoken.find_many( + where={"key_alias": scratch.prefix} + ) if expected_status == 200: - # Allowed: prove the row landed under the scratch namespace. - body_json = resp.json() - cleartext = body_json["key"] + cleartext = resp.json()["key"] assert cleartext.startswith("sk-") - rows = await prisma.db.litellm_verificationtoken.find_many( - where={"key_alias": scratch.prefix} - ) - assert ( - len(rows) == 1 - ), f"{actor.value}: expected exactly one row under scratch, got {len(rows)}" + assert len(rows) == 1 else: - # Denied: prove no row was written. - rows = await prisma.db.litellm_verificationtoken.find_many( - where={"key_alias": scratch.prefix} - ) - assert rows == [], ( - f"{actor.value}: handler returned {expected_status} but row leaked: " - f"{rows[0].token_id if rows else None}" - ) + assert rows == [], f"{actor.value}: denied but row leaked" diff --git a/tests/proxy_behavior/management/test_key_info.py b/tests/proxy_behavior/management/test_key_info.py index 6935d0d3bb9..ddcef9fd27b 100644 --- a/tests/proxy_behavior/management/test_key_info.py +++ b/tests/proxy_behavior/management/test_key_info.py @@ -1,30 +1,20 @@ -"""Slice 8 — actor × target authz matrix for ``GET /key/info``. - -Targets are three fixed seeded keys representing the canonical relations: - - * **own** — the actor's own key (each actor's relation to itself). - * **owner_key** — OWNER actor's key in org_a / team_alpha. From any - other org_a / team_alpha actor's perspective this is "same-team, not - mine". From cross_org_user's perspective this is cross-org. - * **cross_org_user_key** — CROSS_ORG_USER's key in org_b / team_beta. - From any org_a actor's perspective this is cross-org. - -24 (actor × target) scenarios after applying the matrix; status codes pinned -against current handler behavior so future PRs that change the visibility -boundary turn the suite red. -""" - import pytest from .actors import Actor -from .conftest import MASTER_KEY # noqa: F401 (kept for symmetry with sibling files) pytestmark = pytest.mark.asyncio(loop_scope="session") -# (id, actor, target_actor, expected_status) +# (id, actor, target_actor, expected_status). Targets are 3 fixed seeded keys +# representing the canonical relations: own, OWNER (same org_a/team_alpha), +# and CROSS_ORG_USER (org_b/team_beta). +# +# Notable pinned behaviors (intentionally surfaced, not endorsed): +# - ORG_ADMIN 403s on individual key info even within its own org — +# visibility is "your own keys" + "your team's keys", not "your org's keys". +# - Same-team peers (internal_user, unrelated_same_org, service_account) DO +# see each other's keys. _SCENARIOS = [ - # ─── target = own key ───────────────────────────────────────────────── ("own/proxy_admin", Actor.PROXY_ADMIN, Actor.PROXY_ADMIN, 200), ("own/org_admin", Actor.ORG_ADMIN, Actor.ORG_ADMIN, 200), ("own/team_admin", Actor.TEAM_ADMIN, Actor.TEAM_ADMIN, 200), @@ -33,11 +23,6 @@ _SCENARIOS = [ ("own/unrelated_same_org", Actor.UNRELATED_SAME_ORG, Actor.UNRELATED_SAME_ORG, 200), ("own/cross_org_user", Actor.CROSS_ORG_USER, Actor.CROSS_ORG_USER, 200), ("own/service_account", Actor.SERVICE_ACCOUNT, Actor.SERVICE_ACCOUNT, 200), - # ─── target = OWNER's key (org_a / team_alpha) ──────────────────────── - # NB: org_admin currently 403s on individual key info even within their own - # org — visibility is scoped to "your own keys" + "your team's keys", not - # "your org's keys". Same-team peers (internal_user, unrelated_same_org, - # service_account) DO see each other's keys. ("owner_key/proxy_admin", Actor.PROXY_ADMIN, Actor.OWNER, 200), ("owner_key/org_admin", Actor.ORG_ADMIN, Actor.OWNER, 403), ("owner_key/team_admin", Actor.TEAM_ADMIN, Actor.OWNER, 200), @@ -46,7 +31,6 @@ _SCENARIOS = [ ("owner_key/unrelated_same_org", Actor.UNRELATED_SAME_ORG, Actor.OWNER, 200), ("owner_key/cross_org_user", Actor.CROSS_ORG_USER, Actor.OWNER, 403), ("owner_key/service_account", Actor.SERVICE_ACCOUNT, Actor.OWNER, 200), - # ─── target = CROSS_ORG_USER's key (org_b / team_beta) ──────────────── ("cross_org/proxy_admin", Actor.PROXY_ADMIN, Actor.CROSS_ORG_USER, 200), ("cross_org/org_admin", Actor.ORG_ADMIN, Actor.CROSS_ORG_USER, 403), ("cross_org/team_admin", Actor.TEAM_ADMIN, Actor.CROSS_ORG_USER, 403), @@ -69,11 +53,7 @@ _SCENARIOS = [ ids=[s[0] for s in _SCENARIOS], ) async def test_key_info_authz_matrix( - actor: Actor, - target_actor: Actor, - expected_status: int, - proxy_client, - world, + actor: Actor, target_actor: Actor, expected_status: int, proxy_client, world ): caller = world.keys[actor] target = world.keys[target_actor] @@ -82,22 +62,13 @@ async def test_key_info_authz_matrix( f"/key/info?key={target.cleartext}", headers={"Authorization": f"Bearer {caller.cleartext}"}, ) - assert resp.status_code == expected_status, ( - f"{actor.value} GET /key/info?key=<{target_actor.value}> → " - f"{resp.status_code} (expected {expected_status}). body={resp.text}" - ) + assert ( + resp.status_code == expected_status + ), f"{actor.value} → {target_actor.value}: {resp.status_code} {resp.text}" if expected_status == 200: body = resp.json() - # The handler echoes back whatever ``key`` was passed in the query string, - # so body["key"] == cleartext when we querystring it. When the auth key - # IS the target (no query), it returns the hashed form. Either match is - # fine — the canonical identity check is on info.user_id. - assert body.get("key") in (target.cleartext, target.hashed), ( - f"{actor.value} → target {target_actor.value}: wrong key in response " - f"(got {body.get('key')!r}, expected cleartext or hashed of target)" - ) - assert body["info"].get("user_id") == target.user_id, ( - f"{actor.value} → target {target_actor.value}: wrong user_id " - f"(got {body['info'].get('user_id')!r}, expected {target.user_id!r})" - ) + # The handler echoes back whatever ?key was passed (cleartext here), + # so accept either form — info.user_id is the canonical identity check. + assert body.get("key") in (target.cleartext, target.hashed) + assert body["info"].get("user_id") == target.user_id diff --git a/tests/proxy_behavior/management/test_key_list.py b/tests/proxy_behavior/management/test_key_list.py index 8cbb21b05fd..bfcd97c66a6 100644 --- a/tests/proxy_behavior/management/test_key_list.py +++ b/tests/proxy_behavior/management/test_key_list.py @@ -1,15 +1,3 @@ -"""Slice 9 — actor visibility matrix for ``GET /key/list``. - -For ``/key/list`` the *response itself* is the matrix: each actor calls the -endpoint with default filters and we assert which seeded actor keys end up in -the returned set. The set-equality assertion is filtered to seeded tokens -only, so unrelated rows in the DB (other tests, leftover dev data) can't flap -the matrix. - -8 scenarios — one per actor. Expected visibility is pinned against the -current handler so future changes to the filter logic surface red. -""" - from typing import FrozenSet import pytest @@ -19,8 +7,8 @@ from .actors import Actor pytestmark = pytest.mark.asyncio(loop_scope="session") -# Maps each actor → the set of seeded actors whose keys it is permitted to see -# under default ``/key/list`` (no filter params, page=1, size=10). +# Pinned default visibility for /key/list (no filter params): each actor's +# expected set of seeded actor keys. _VISIBILITY = { Actor.PROXY_ADMIN: frozenset(Actor), Actor.ORG_ADMIN: frozenset({Actor.ORG_ADMIN}), @@ -39,46 +27,25 @@ _VISIBILITY = { ids=[a.value for a in _VISIBILITY], ) async def test_key_list_visibility( - actor: Actor, - expected_visible: FrozenSet[Actor], - proxy_client, - world, + actor: Actor, expected_visible: FrozenSet[Actor], proxy_client, world ): caller = world.keys[actor] - seeded_hashes = {a: world.keys[a].hashed for a in Actor} - hashed_to_actor = {h: a for a, h in seeded_hashes.items()} + hashed_to_actor = {world.keys[a].hashed: a for a in Actor} - # Use size=100 to ensure we see all 8 seeded keys for proxy_admin. resp = await proxy_client.get( "/key/list?size=100", headers={"Authorization": f"Bearer {caller.cleartext}"}, ) - assert ( - resp.status_code == 200 - ), f"{actor.value} GET /key/list → {resp.status_code}: {resp.text}" - - body = resp.json() - returned = body.get("keys", []) - - # /key/list can return either token strings (default) or full objects. - # Default: list of dicts with ``token`` key. Reduce to hashes. - returned_hashes = set() - for entry in returned: - if isinstance(entry, dict): - tok = entry.get("token") - else: - tok = entry - if tok: - returned_hashes.add(tok) + 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", []) + } visible_seeded = { hashed_to_actor[h] for h in returned_hashes if h in hashed_to_actor } - expected = set(expected_visible) - assert visible_seeded == expected, ( - f"{actor.value} /key/list visibility differs:\n" - f" expected: {sorted(a.value for a in expected)}\n" - f" actual: {sorted(a.value for a in visible_seeded)}\n" - f" diff (missing): {sorted(a.value for a in (expected - visible_seeded))}\n" - f" diff (extra): {sorted(a.value for a in (visible_seeded - expected))}" + assert visible_seeded == set(expected_visible), ( + f"{actor.value}: expected {sorted(a.value for a in expected_visible)}, " + f"got {sorted(a.value for a in visible_seeded)}" ) diff --git a/tests/proxy_behavior/management/test_key_regenerate.py b/tests/proxy_behavior/management/test_key_regenerate.py index cbe677c7926..906dcc9c7be 100644 --- a/tests/proxy_behavior/management/test_key_regenerate.py +++ b/tests/proxy_behavior/management/test_key_regenerate.py @@ -1,16 +1,3 @@ -"""Slice 11 — actor × target authz matrix for ``POST /key/regenerate``. - -Mirrors Slice 10's shape: each test master-seeds a fresh scratch key with the -target's scope, then the actor attempts to regenerate it. On success the test -asserts both routes-of-the-rotation contract: - - * the OLD cleartext can no longer authenticate (``/key/info`` with it 401s), - * the NEW cleartext (returned in the regenerate response) IS accepted. - -A separate single-actor smoke covers the ``/key/{key:path}/regenerate`` route -form to prove both registrations exercise the same handler. -""" - from typing import Any, Dict, Optional import pytest @@ -20,9 +7,10 @@ from .actors import TEAM_ALPHA, TEAM_BETA, Actor pytestmark = pytest.mark.asyncio(loop_scope="session") -# (id, actor, target_shape, expected_status) +# Most denials route through team_member_permission (401), unlike /key/update +# which goes through user_id-mismatch (403). The matrix surfaces that +# divergence between the two endpoints. _SCENARIOS = [ - # ─── target = self-owned key ────────────────────────────────────────── ("self/proxy_admin", Actor.PROXY_ADMIN, "self", 200), ("self/org_admin", Actor.ORG_ADMIN, "self", 401), ("self/team_admin", Actor.TEAM_ADMIN, "self", 200), @@ -31,10 +19,6 @@ _SCENARIOS = [ ("self/unrelated_same_org", Actor.UNRELATED_SAME_ORG, "self", 200), ("self/cross_org_user", Actor.CROSS_ORG_USER, "self", 200), ("self/service_account", Actor.SERVICE_ACCOUNT, "self", 200), - # ─── target = OWNER-scoped key in org_a / team_alpha ────────────────── - # NB: /key/regenerate routes most denials through the team-member-perm - # path (401), not the same 403 paths /key/update uses. This is observable - # divergence between the two endpoints — the matrix surfaces it. ("owner_target/proxy_admin", Actor.PROXY_ADMIN, "owner", 200), ("owner_target/org_admin", Actor.ORG_ADMIN, "owner", 401), ("owner_target/team_admin", Actor.TEAM_ADMIN, "owner", 200), @@ -42,12 +26,10 @@ _SCENARIOS = [ ("owner_target/unrelated_same_org", Actor.UNRELATED_SAME_ORG, "owner", 401), ("owner_target/cross_org_user", Actor.CROSS_ORG_USER, "owner", 401), ("owner_target/service_account", Actor.SERVICE_ACCOUNT, "owner", 401), - # ─── target = CROSS_ORG_USER-scoped key in org_b / team_beta ────────── ("cross_org_target/proxy_admin", Actor.PROXY_ADMIN, "cross_org", 200), ("cross_org_target/org_admin", Actor.ORG_ADMIN, "cross_org", 401), ("cross_org_target/team_admin", Actor.TEAM_ADMIN, "cross_org", 401), ("cross_org_target/owner", Actor.OWNER, "cross_org", 401), - # cross_org_user is in team_beta as a non-admin → 401 team_member_permission. ("cross_org_target/cross_org_user", Actor.CROSS_ORG_USER, "cross_org", 401), ("cross_org_target/service_account", Actor.SERVICE_ACCOUNT, "cross_org", 401), ] @@ -61,7 +43,6 @@ async def _create_scratch_key( user_id: str, team_id: Optional[str] = None, ) -> str: - """Seeded under the proxy_admin actor (deterministic PROXY_ADMIN bypass).""" body: Dict[str, Any] = {"key_alias": scratch_prefix, "user_id": user_id} if team_id is not None: body["team_id"] = team_id @@ -70,10 +51,16 @@ async def _create_scratch_key( headers={"Authorization": f"Bearer {seeder_cleartext}"}, json=body, ) - assert resp.status_code == 200, f"setup: seeder /key/generate failed: {resp.text}" + 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}"} + ) + + @pytest.mark.parametrize( "actor,target_shape,expected_status", [(a, t, s) for (_id, a, t, s) in _SCENARIOS], @@ -88,8 +75,8 @@ async def test_key_regenerate_authz_matrix( world, ): caller = world.keys[actor] - seeder = world.keys[Actor.PROXY_ADMIN].cleartext + if target_shape == "self": target_cleartext = await _create_scratch_key( proxy_client, seeder, scratch.prefix, user_id=caller.user_id @@ -118,50 +105,22 @@ async def test_key_regenerate_authz_matrix( headers={"Authorization": f"Bearer {caller.cleartext}"}, json={"key": target_cleartext}, ) - assert resp.status_code == expected_status, ( - f"{actor.value} POST /key/regenerate {target_shape} → " - f"{resp.status_code} (expected {expected_status}). body={resp.text}" - ) + assert ( + resp.status_code == expected_status + ), f"{actor.value} {target_shape}: {resp.status_code} {resp.text}" if expected_status == 200: new_cleartext = resp.json()["key"] - assert ( - new_cleartext.startswith("sk-") and new_cleartext != target_cleartext - ), "regenerate returned the same cleartext — rotation contract broken" - # Old cleartext no longer authenticates. - old_check = await proxy_client.get( - "/key/info", - headers={"Authorization": f"Bearer {target_cleartext}"}, - ) - assert old_check.status_code == 401, ( - f"old cleartext still works post-regenerate: {old_check.status_code} " - f"{old_check.text}" - ) - # New cleartext does authenticate. - new_check = await proxy_client.get( - "/key/info", - headers={"Authorization": f"Bearer {new_cleartext}"}, - ) - assert new_check.status_code == 200, ( - f"new cleartext rejected post-regenerate: {new_check.status_code} " - f"{new_check.text}" - ) + assert new_cleartext.startswith("sk-") and new_cleartext != target_cleartext + assert (await _info(proxy_client, target_cleartext)).status_code == 401 + assert (await _info(proxy_client, new_cleartext)).status_code == 200 else: - # Denied: old cleartext should still work. - check = await proxy_client.get( - "/key/info", - headers={"Authorization": f"Bearer {target_cleartext}"}, - ) - assert check.status_code == 200, ( - f"{actor.value}: handler returned {expected_status} but old cleartext " - f"is no longer valid — rotation may have leaked: {check.text}" - ) + # Denied: rotation must not have leaked — old cleartext still works. + assert (await _info(proxy_client, target_cleartext)).status_code == 200 async def test_key_path_regenerate_smoke(proxy_client, scratch, world): - """Confirms the ``POST /key/{key:path}/regenerate`` registration shares the - same handler — proxy_admin regenerates a self-owned scratch key via the - path form, and the same rotation contract holds.""" + """Pins that POST /key/{key:path}/regenerate shares the same handler.""" caller = world.keys[Actor.PROXY_ADMIN] target_cleartext = await _create_scratch_key( proxy_client, caller.cleartext, scratch.prefix, user_id=caller.user_id @@ -175,14 +134,5 @@ async def test_key_path_regenerate_smoke(proxy_client, scratch, world): assert resp.status_code == 200, resp.text new_cleartext = resp.json()["key"] assert new_cleartext.startswith("sk-") and new_cleartext != target_cleartext - - # Old denied, new accepted. - old_check = await proxy_client.get( - "/key/info", headers={"Authorization": f"Bearer {target_cleartext}"} - ) - assert old_check.status_code == 401 - - new_check = await proxy_client.get( - "/key/info", headers={"Authorization": f"Bearer {new_cleartext}"} - ) - assert new_check.status_code == 200 + assert (await _info(proxy_client, target_cleartext)).status_code == 401 + assert (await _info(proxy_client, new_cleartext)).status_code == 200 diff --git a/tests/proxy_behavior/management/test_key_update.py b/tests/proxy_behavior/management/test_key_update.py index 6520fbfa555..d0dc401feef 100644 --- a/tests/proxy_behavior/management/test_key_update.py +++ b/tests/proxy_behavior/management/test_key_update.py @@ -1,49 +1,19 @@ -"""Slice 10 — actor × target authz matrix for ``POST /key/update``. - -Each scenario: - 1. The master key seeds a fresh scratch key with a specific (user_id, - team_id, organization_id) scope on it. - 2. The actor under test calls ``POST /key/update`` to flip its ``models`` - field to a known marker list. - 3. The test asserts the status code and (when 200) that the mutation - actually landed in the DB. - -Three target shapes: - * **self**: target is scoped to the actor's own user (the actor's "own" key). - * **owner_target**: scoped to OWNER in org_a / team_alpha — the same-team / - same-org / different-user target. - * **cross_org_target**: scoped to CROSS_ORG_USER in org_b / team_beta — the - cross-org target. - -Status codes are pinned against observed handler behavior. -""" - -from typing import Any, Dict, List +from typing import Any, Dict, Optional import pytest +from litellm.proxy.utils import hash_token + from .actors import TEAM_ALPHA, TEAM_BETA, Actor pytestmark = pytest.mark.asyncio(loop_scope="session") -# (id, actor, target_shape, expected_status) -# -# Pinned against current /key/update gating: -# -# * PROXY_ADMIN bypasses every check (200). -# * ORG_ADMIN is blocked by an early role gate, regardless of target -# scope, with a 401 "Only proxy admin..." auth_error. -# * Every other (INTERNAL_USER-rolesed) actor hits one of: -# - 403 "User can only create keys for themselves" when the target key's -# user_id is not theirs; -# - 403 "Only proxy admins, team admins, or org admins can call /key/update" -# when they own the target but lack team-admin status on the target -# key's team (or the key has no team); -# - 401 "Team member does not have permissions" when they own the target -# in a team they're a member of without admin role. +# (id, actor, target_shape, expected_status). Pinned against current gating: +# proxy_admin bypasses; org_admin is blocked by an early role gate (401); +# every other (INTERNAL_USER-roled) actor hits user_id-mismatch 403, no-team- +# admin 403, or team_member_permission 401 depending on target / membership. _SCENARIOS = [ - # ─── target = self-owned key (no team_id on the key) ────────────────── ("self/proxy_admin", Actor.PROXY_ADMIN, "self", 200), ("self/org_admin", Actor.ORG_ADMIN, "self", 401), ("self/team_admin", Actor.TEAM_ADMIN, "self", 403), @@ -52,23 +22,17 @@ _SCENARIOS = [ ("self/unrelated_same_org", Actor.UNRELATED_SAME_ORG, "self", 403), ("self/cross_org_user", Actor.CROSS_ORG_USER, "self", 403), ("self/service_account", Actor.SERVICE_ACCOUNT, "self", 403), - # ─── target = OWNER-scoped key in org_a / team_alpha ────────────────── ("owner_target/proxy_admin", Actor.PROXY_ADMIN, "owner", 200), ("owner_target/org_admin", Actor.ORG_ADMIN, "owner", 401), - # team_admin gets 403 "user can only create keys for themselves" — the - # user_id-mismatch check fires BEFORE the team-admin check. ("owner_target/team_admin", Actor.TEAM_ADMIN, "owner", 403), ("owner_target/internal_user", Actor.INTERNAL_USER, "owner", 403), ("owner_target/unrelated_same_org", Actor.UNRELATED_SAME_ORG, "owner", 403), ("owner_target/cross_org_user", Actor.CROSS_ORG_USER, "owner", 403), ("owner_target/service_account", Actor.SERVICE_ACCOUNT, "owner", 403), - # ─── target = CROSS_ORG_USER-scoped key in org_b / team_beta ────────── ("cross_org_target/proxy_admin", Actor.PROXY_ADMIN, "cross_org", 200), ("cross_org_target/org_admin", Actor.ORG_ADMIN, "cross_org", 401), ("cross_org_target/team_admin", Actor.TEAM_ADMIN, "cross_org", 403), ("cross_org_target/owner", Actor.OWNER, "cross_org", 403), - # cross_org_user owns the target (user_id matches) and is in team_beta as - # a non-admin → team_member_permission_error 401. ("cross_org_target/cross_org_user", Actor.CROSS_ORG_USER, "cross_org", 401), ("cross_org_target/service_account", Actor.SERVICE_ACCOUNT, "cross_org", 403), ] @@ -82,16 +46,9 @@ async def _create_scratch_key( scratch_prefix: str, *, user_id: str, - team_id: str = None, - organization_id: str = None, + team_id: Optional[str] = None, + organization_id: Optional[str] = None, ) -> str: - """Seed a fresh key tagged with scratch_prefix using the proxy_admin actor. - - Using the seeded PROXY_ADMIN actor (not the bare master key) makes the - setup call's auth path deterministic: user_role=PROXY_ADMIN + a concrete - user_id, which reliably triggers - ``_user_can_only_create_keys_for_themselves``'s admin bypass. - """ body: Dict[str, Any] = {"key_alias": scratch_prefix, "user_id": user_id} if team_id is not None: body["team_id"] = team_id @@ -102,7 +59,7 @@ async def _create_scratch_key( headers={"Authorization": f"Bearer {seeder_cleartext}"}, json=body, ) - assert resp.status_code == 200, f"setup: seeder /key/generate failed: {resp.text}" + assert resp.status_code == 200, f"setup failed: {resp.text}" return resp.json()["key"] @@ -121,8 +78,8 @@ async def test_key_update_authz_matrix( world, ): caller = world.keys[actor] - seeder = world.keys[Actor.PROXY_ADMIN].cleartext + if target_shape == "self": target_cleartext = await _create_scratch_key( proxy_client, seeder, scratch.prefix, user_id=caller.user_id @@ -146,8 +103,6 @@ async def test_key_update_authz_matrix( else: pytest.fail(f"unknown target_shape={target_shape}") - from litellm.proxy.utils import hash_token - target_hashed = hash_token(target_cleartext) resp = await proxy_client.post( @@ -155,23 +110,15 @@ async def test_key_update_authz_matrix( headers={"Authorization": f"Bearer {caller.cleartext}"}, json={"key": target_cleartext, "models": [MARKER_MODEL]}, ) - assert resp.status_code == expected_status, ( - f"{actor.value} POST /key/update {target_shape} → {resp.status_code} " - f"(expected {expected_status}). body={resp.text}" - ) + assert ( + resp.status_code == expected_status + ), f"{actor.value} {target_shape}: {resp.status_code} {resp.text}" row = await prisma.db.litellm_verificationtoken.find_unique( where={"token": target_hashed} ) - assert row is not None, "scratch key vanished mid-test" - + assert row is not None if expected_status == 200: - assert row.models == [MARKER_MODEL], ( - f"{actor.value}: handler returned 200 but row.models did not flip — " - f"got {row.models!r}" - ) + assert row.models == [MARKER_MODEL] else: - assert row.models != [MARKER_MODEL], ( - f"{actor.value}: handler returned {expected_status} but row.models was " - f"mutated to {row.models!r}" - ) + assert row.models != [MARKER_MODEL], "denied but row mutated" diff --git a/tests/proxy_behavior/management/test_no_management_imports.py b/tests/proxy_behavior/management/test_no_management_imports.py index 39f871aa556..f8c52a1c37e 100644 --- a/tests/proxy_behavior/management/test_no_management_imports.py +++ b/tests/proxy_behavior/management/test_no_management_imports.py @@ -1,25 +1,6 @@ -"""Codify G3 (strict-import grep) as a test. - -The behavior-pinning suite asserts at the HTTP boundary against a real proxy -app. Two forbidden patterns: - -1. ``from litellm.proxy.management_endpoints`` — importing handler functions - directly turns the suite into unit tests of the handler module rather than - behavior tests of the API, and makes the suite brittle to refactors. - -2. ``mock``/``patch`` on ``user_api_key_auth`` — mocking auth is the - structural failure mode of today's mock-heavy suite. The whole point of the - real-DB harness is that auth runs. - -Running this as a pytest item means G3 is enforced by CI on every PR, not by -a checklist someone might forget. -""" - import pathlib import re -import pytest - REPO_ROOT = pathlib.Path(__file__).resolve().parents[3] BEHAVIOR_DIR = REPO_ROOT / "tests" / "proxy_behavior" @@ -27,42 +8,39 @@ FORBIDDEN_IMPORT = re.compile(r"^\s*from\s+litellm\.proxy\.management_endpoints\ FORBIDDEN_AUTH_MOCK = re.compile( r"(?:mock\.[A-Za-z_]+|patch[a-z_]*)\([^)]*user_api_key_auth" ) - -# This very file is the one place where the forbidden patterns appear as -# regex source; exclude it from its own scan so the test is self-checkable. +# This file is the only place the forbidden patterns appear as regex source; +# exclude it so it can describe what it forbids. SELF = pathlib.Path(__file__).resolve() def _iter_py_files(): for path in BEHAVIOR_DIR.rglob("*.py"): - if path.resolve() == SELF: - continue - yield path + if path.resolve() != SELF: + yield path -def test_no_management_endpoint_imports(): +def _scan(pattern): violations = [] for path in _iter_py_files(): for lineno, line in enumerate(path.read_text().splitlines(), start=1): - if FORBIDDEN_IMPORT.search(line): + if pattern.search(line): violations.append( f"{path.relative_to(REPO_ROOT)}:{lineno}: {line.strip()}" ) + return violations + + +def test_no_management_endpoint_imports(): + violations = _scan(FORBIDDEN_IMPORT) assert not violations, ( - "tests/proxy_behavior/ must not import from litellm.proxy.management_endpoints " - "(G3 — assert at the HTTP boundary). Violations:\n " + "\n ".join(violations) + "tests/proxy_behavior/ must not import from litellm.proxy.management_endpoints. " + "Violations:\n " + "\n ".join(violations) ) def test_no_user_api_key_auth_mocking(): - violations = [] - for path in _iter_py_files(): - for lineno, line in enumerate(path.read_text().splitlines(), start=1): - if FORBIDDEN_AUTH_MOCK.search(line): - violations.append( - f"{path.relative_to(REPO_ROOT)}:{lineno}: {line.strip()}" - ) + violations = _scan(FORBIDDEN_AUTH_MOCK) assert not violations, ( - "tests/proxy_behavior/ must not mock user_api_key_auth (G3 — auth runs for " - "real). Violations:\n " + "\n ".join(violations) + "tests/proxy_behavior/ must not mock user_api_key_auth. " + "Violations:\n " + "\n ".join(violations) ) diff --git a/tests/proxy_behavior/management/test_scratch_teardown.py b/tests/proxy_behavior/management/test_scratch_teardown.py index 1f6bad64539..689c60fc78a 100644 --- a/tests/proxy_behavior/management/test_scratch_teardown.py +++ b/tests/proxy_behavior/management/test_scratch_teardown.py @@ -1,12 +1,3 @@ -"""Slice 5 smoke: scratch namespace fixture cleans up after itself. - -Two ordered tests. The first writes a key tagged with the scratch prefix and -asserts it lands. The second runs after the first's teardown and asserts no -scratch-namespaced rows survived. Together they prove the per-test cleanup -filter is the right shape — any leaked row will surface as a test-2 failure on -the very next run. -""" - import pytest from .conftest import MASTER_KEY, SCRATCH_PREFIX @@ -14,6 +5,11 @@ from .conftest import MASTER_KEY, SCRATCH_PREFIX pytestmark = pytest.mark.asyncio(loop_scope="session") +# The two tests run in file order: _a writes a scratch-tagged key and asserts +# it lands; _b runs after _a's fixture teardown and asserts no scratch row +# survived. A leak in either direction fails _b on the next collection. + + async def test_a_scratch_key_lands_in_db(proxy_client, prisma, scratch): resp = await proxy_client.post( "/key/generate", @@ -25,17 +21,11 @@ async def test_a_scratch_key_lands_in_db(proxy_client, prisma, scratch): rows = await prisma.db.litellm_verificationtoken.find_many( where={"key_alias": scratch.prefix} ) - assert ( - len(rows) == 1 - ), f"expected exactly one scratch-tagged token, found {len(rows)}" + assert len(rows) == 1 async def test_b_scratch_namespace_is_clean(prisma): - """Runs after test_a's teardown — proves nothing leaked.""" rows = await prisma.db.litellm_verificationtoken.find_many( where={"key_alias": {"startswith": SCRATCH_PREFIX}} ) - assert rows == [], ( - f"scratch teardown leaked {len(rows)} rows; first key_alias: " - f"{rows[0].key_alias if rows else None!r}" - ) + assert rows == [] diff --git a/tests/proxy_behavior/management/test_smoke.py b/tests/proxy_behavior/management/test_smoke.py index 5675a99606c..4e90986ad9f 100644 --- a/tests/proxy_behavior/management/test_smoke.py +++ b/tests/proxy_behavior/management/test_smoke.py @@ -1,5 +1,3 @@ -"""Smoke tests proving the harness boots and talks to the proxy app.""" - import pytest from .conftest import MASTER_KEY @@ -13,19 +11,6 @@ async def test_liveliness(proxy_client): async def test_key_generate_lands_in_db(proxy_client, prisma, scratch): - """De-risk gate: prove the harness exercises the full stack end-to-end. - - A successful ``/key/generate`` requires: - * the FastAPI lifespan ran (``proxy_startup_event``), - * ``prisma_client`` connected, - * ``user_api_key_auth`` accepted the master key, - * the real ``generate_key_helper_fn`` wrote a hashed row to - ``LiteLLM_VerificationToken``. - - The scratch fixture tags the row with its prefix so the per-test teardown - cleans it up — keeps the proxy DB free of accumulated cruft on repeated - local runs. - """ from litellm.proxy.utils import hash_token resp = await proxy_client.post( @@ -34,14 +19,10 @@ async def test_key_generate_lands_in_db(proxy_client, prisma, scratch): json={"key_alias": scratch.prefix}, ) assert resp.status_code == 200, resp.text - body = resp.json() - cleartext_key = body["key"] - assert cleartext_key.startswith("sk-") + cleartext = resp.json()["key"] + assert cleartext.startswith("sk-") - hashed = hash_token(cleartext_key) + hashed = hash_token(cleartext) row = await prisma.db.litellm_verificationtoken.find_unique(where={"token": hashed}) - assert row is not None, "Generated key did not land in LiteLLM_VerificationToken" - assert row.token == hashed - assert ( - row.token != cleartext_key - ), "Cleartext token stored — credential boundary broken" + assert row is not None + assert row.token == hashed != cleartext diff --git a/tests/proxy_behavior/management/test_world_seed.py b/tests/proxy_behavior/management/test_world_seed.py index 8a32fe3819e..00f9540c9c3 100644 --- a/tests/proxy_behavior/management/test_world_seed.py +++ b/tests/proxy_behavior/management/test_world_seed.py @@ -1,19 +1,3 @@ -"""Slice 4 smoke: every seeded actor key can authenticate and call /key/info on itself. - -Minimal proof that the world is reachable via the real auth stack — the real -``user_api_key_auth`` dependency hashes the cleartext token from the Bearer -header, looks it up, resolves the user / role, and the handler returns the -key info row. - -If any actor 401/403s here, the seed is wrong (user_role mismatch, scope -fields, etc.) before we scale out to the matrix in Slices 7–12. - -Also pins the PROXY_ADMIN bypass contract for ``/key/generate`` so we don't -regress on the master-key / lifespan env-var interaction that surfaced -during CI bring-up (see the conftest's ``LITELLM_MASTER_KEY`` note for the -underlying mechanism). -""" - import pytest from .actors import Actor @@ -28,30 +12,13 @@ async def test_each_actor_can_self_info(actor, proxy_client, world): "/key/info", headers={"Authorization": f"Bearer {seeded.cleartext}"}, ) - assert resp.status_code == 200, f"{actor.value}: {resp.status_code} {resp.text}" + assert resp.status_code == 200, f"{actor.value}: {resp.text}" body = resp.json() - # /key/info returns {"key": , "info": }. - assert body.get("key") == seeded.hashed, ( - f"{actor.value}: /key/info returned the wrong key " - f"(got {body.get('key')!r}, expected {seeded.hashed!r})" - ) - info = body["info"] - assert info.get("user_id") == seeded.user_id, ( - f"{actor.value}: /key/info returned the wrong user_id " - f"(got {info.get('user_id')!r}, expected {seeded.user_id!r})" - ) + assert body.get("key") == seeded.hashed + assert body["info"].get("user_id") == seeded.user_id async def test_proxy_admin_actor_can_create_keys_for_others(proxy_client, world): - """Pins the PROXY_ADMIN bypass for /key/generate's user_id-mismatch gate. - - The seeded PROXY_ADMIN actor must be able to /key/generate a key with an - explicit ``user_id`` belonging to a different user. The bypass relies on - user_api_key_auth resolving user_role=PROXY_ADMIN from the user-table row - keyed off the verification token's user_id. If any link in that chain - breaks, this test surfaces it directly rather than as cascading failures - in the write matrices that depend on the same setup helper. - """ seeder = world.keys[Actor.PROXY_ADMIN] target_user_id = world.keys[Actor.OWNER].user_id @@ -60,7 +27,4 @@ async def test_proxy_admin_actor_can_create_keys_for_others(proxy_client, world) headers={"Authorization": f"Bearer {seeder.cleartext}"}, json={"key_alias": "smoke-proxy-admin-bypass", "user_id": target_user_id}, ) - assert resp.status_code == 200, ( - f"PROXY_ADMIN-seeded actor can't create keys for others: " - f"{resp.status_code} {resp.text}" - ) + assert resp.status_code == 200, resp.text