From 4c66635ee5f79e36bed040cb214205128f50583f Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Wed, 20 May 2026 13:38:42 -0700 Subject: [PATCH] test(proxy_behavior): trim debug diagnostics, restore default max-failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Followup to the CI-bring-up sequence: now that the suite is green in CI (130 → 129 tests after this trim; 156s wall-time on ubuntu-latest), drop the diagnostic noise left over from debugging the master_key wipe: * Rename ``test_aaa_world_seed.py`` back to ``test_world_seed.py`` — no longer needs to run first. * Remove ``test_auth_resolver_returns_correct_user_id_and_role`` — that test reached into private auth helpers to localize the bug between the DB and ``UserAPIKeyAuth``; it has served its purpose and isn't HTTP-boundary. * Keep ``test_proxy_admin_actor_can_create_keys_for_others`` (without the failure-time dump) — it's a real authz contract that pins the PROXY_ADMIN bypass on /key/generate, and would catch a regression of the same conftest interaction this sequence revealed. * Drop the workflow's ``max-failures: 200`` override — that was a debug aid for seeing the full failure surface in CI. Default of 10 is right for a stable suite. --- .../test-unit-proxy-mgmt-behavior.yml | 4 - .../management/test_aaa_world_seed.py | 152 ------------------ .../management/test_world_seed.py | 66 ++++++++ 3 files changed, 66 insertions(+), 156 deletions(-) delete mode 100644 tests/proxy_behavior/management/test_aaa_world_seed.py create mode 100644 tests/proxy_behavior/management/test_world_seed.py diff --git a/.github/workflows/test-unit-proxy-mgmt-behavior.yml b/.github/workflows/test-unit-proxy-mgmt-behavior.yml index 38b606901ae..e73997323a4 100644 --- a/.github/workflows/test-unit-proxy-mgmt-behavior.yml +++ b/.github/workflows/test-unit-proxy-mgmt-behavior.yml @@ -29,10 +29,6 @@ jobs: # so the cost of disabling parallelism here is negligible. workers: 0 reruns: 0 - # Don't abort early — first CI runs need the full failure surface so we - # can correlate setup-helper failures (one bad fixture cascades to N - # tests) vs. real per-scenario failures. Will tighten back down later. - max-failures: 200 enable-postgres: true artifact-name: proxy-mgmt-behavior timeout-minutes: 15 diff --git a/tests/proxy_behavior/management/test_aaa_world_seed.py b/tests/proxy_behavior/management/test_aaa_world_seed.py deleted file mode 100644 index 9633de77529..00000000000 --- a/tests/proxy_behavior/management/test_aaa_world_seed.py +++ /dev/null @@ -1,152 +0,0 @@ -"""Slice 4 smoke: every seeded actor key can authenticate and call /key/info on itself. - -This is the minimum 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. -""" - -import pytest - -from .actors import Actor - -pytestmark = pytest.mark.asyncio(loop_scope="session") - - -@pytest.mark.parametrize("actor", list(Actor), ids=[a.value for a in Actor]) -async def test_each_actor_can_self_info(actor, proxy_client, world): - seeded = world.keys[actor] - resp = await proxy_client.get( - "/key/info", - headers={"Authorization": f"Bearer {seeded.cleartext}"}, - ) - assert resp.status_code == 200, f"{actor.value}: {resp.status_code} {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})" - ) - - -async def test_auth_resolver_returns_correct_user_id_and_role( - proxy_client, prisma, world -): - """Diagnostic: call the auth-resolver primitives directly with the seeded - PROXY_ADMIN actor's hashed token, dump every intermediate so we can see - where in the chain user_id / user_role get dropped between the DB and - UserAPIKeyAuth.""" - from litellm.proxy.auth.auth_checks import ( - get_key_object, - get_user_object, - _is_user_proxy_admin, - _get_user_role, - ) - from litellm.proxy.proxy_server import user_api_key_cache - - seeder = world.keys[Actor.PROXY_ADMIN] - - # 1. Raw prisma get_data → LiteLLM_VerificationTokenView - raw_view = await prisma.get_data(token=seeder.hashed, table_name="combined_view") - raw_view_user_id = getattr(raw_view, "user_id", "") - - # 2. get_key_object (cache miss → DB) - key_obj = await get_key_object( - hashed_token=seeder.hashed, - prisma_client=prisma, - user_api_key_cache=user_api_key_cache, - ) - key_user_id = getattr(key_obj, "user_id", "") - key_user_role = getattr(key_obj, "user_role", "") - - # 3. get_user_object for the resolved user_id - user_obj = None - if key_user_id and key_user_id != "": - try: - user_obj = await get_user_object( - user_id=key_user_id, - prisma_client=prisma, - user_api_key_cache=user_api_key_cache, - user_id_upsert=False, - ) - except Exception as exc: - user_obj = f"" - - is_admin = ( - _is_user_proxy_admin(user_obj=user_obj) - if not isinstance(user_obj, str) - else "" - ) - resolved_role = ( - _get_user_role(user_obj=user_obj) if not isinstance(user_obj, str) else "" - ) - - diagnostic = ( - f"\n expected seed.user_id : {seeder.user_id!r}" - f"\n raw_view.user_id : {raw_view_user_id!r}" - f"\n key_obj.user_id : {key_user_id!r}" - f"\n key_obj.user_role : {key_user_role!r}" - f"\n user_obj.user_role : {getattr(user_obj, 'user_role', user_obj)!r}" - f"\n _is_user_proxy_admin : {is_admin!r}" - f"\n _get_user_role : {resolved_role!r}" - ) - print(f"AUTH RESOLVER DIAGNOSTIC:{diagnostic}") - - assert ( - key_user_id == seeder.user_id - ), f"get_key_object dropped user_id: {diagnostic}" - assert ( - is_admin is True - ), f"_is_user_proxy_admin returned False for the seeded PROXY_ADMIN: {diagnostic}" - - -async def test_proxy_admin_actor_can_create_keys_for_others( - proxy_client, prisma, world -): - """Diagnostic: the seeded PROXY_ADMIN actor must be able to /key/generate - a key for another user. If this fails, the user_role is not propagating - through user_api_key_auth → the actor's auth context disagrees with the - DB row, and the cause is elsewhere in the auth stack (not the seed). - - On failure we also probe the underlying state so the CI log tells us - exactly which surface returned the wrong shape: the raw token row, the - user row, and the combined view (what the auth resolver consumes).""" - seeder = world.keys[Actor.PROXY_ADMIN] - target_user_id = world.keys[Actor.OWNER].user_id - - resp = await proxy_client.post( - "/key/generate", - headers={"Authorization": f"Bearer {seeder.cleartext}"}, - json={"key_alias": "diag-proxy-admin-seeder", "user_id": target_user_id}, - ) - if resp.status_code != 200: - token_row = await prisma.db.litellm_verificationtoken.find_unique( - where={"token": seeder.hashed} - ) - user_row = await prisma.db.litellm_usertable.find_unique( - where={"user_id": seeder.user_id} - ) - # View columns: the auth resolver reads from this view, then does a - # separate user-table lookup keyed off the view's user_id to populate - # user_role. So we want both surfaces independently. - view_rows = await prisma.db.query_raw( - "SELECT user_id, team_id, organization_id " - 'FROM "LiteLLM_VerificationTokenView" WHERE token = $1', - seeder.hashed, - ) - pytest.fail( - f"PROXY_ADMIN-seeded actor can't create keys for others: " - f"{resp.status_code} {resp.text}\n" - f" seeder user_id (expected): {seeder.user_id}\n" - f" token row.user_id : {getattr(token_row, 'user_id', '')!r}\n" - f" user row.user_role : {getattr(user_row, 'user_role', '')!r}\n" - f" view row : {view_rows!r}" - ) diff --git a/tests/proxy_behavior/management/test_world_seed.py b/tests/proxy_behavior/management/test_world_seed.py new file mode 100644 index 00000000000..8a32fe3819e --- /dev/null +++ b/tests/proxy_behavior/management/test_world_seed.py @@ -0,0 +1,66 @@ +"""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 + +pytestmark = pytest.mark.asyncio(loop_scope="session") + + +@pytest.mark.parametrize("actor", list(Actor), ids=[a.value for a in Actor]) +async def test_each_actor_can_self_info(actor, proxy_client, world): + seeded = world.keys[actor] + resp = await proxy_client.get( + "/key/info", + headers={"Authorization": f"Bearer {seeded.cleartext}"}, + ) + assert resp.status_code == 200, f"{actor.value}: {resp.status_code} {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})" + ) + + +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 + + resp = await proxy_client.post( + "/key/generate", + 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}" + )