From 12fe945e7bbac2b9e7db6dbcefb73e9a4c5f0a13 Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Mon, 4 May 2026 23:54:33 +0000 Subject: [PATCH] fix: keep skills handler FastAPI-free; fold gcs deny list into the body bouncer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two cleanups: * ``LiteLLMSkillsHandler.create_skill`` raised ``HTTPException`` for identity-less callers, importing FastAPI from a ``litellm/llms/`` module — that violates the project rule that FastAPI lives only under ``proxy/``. Switch to ``ValueError`` (the same shape the rest of the handler uses for not-found/forbidden) and update the test. * The proxy-auth body bouncer derived its observability ban list from ``_supported_callback_params`` only, missing ``_request_blocked_callback_params`` (where ``gcs_bucket_name`` and ``gcs_path_service_account`` live). Two recently-merged sibling PRs (#27019 added the deny list, #27081 added the test asserting these are rejected at the request body root) crossed without folding them together. Union the GCS deny list into the bouncer's derivation so the single source of truth covers both code paths. --- litellm/llms/litellm_proxy/skills/handler.py | 11 +++++------ litellm/proxy/auth/auth_utils.py | 13 +++++++++++-- .../llms/litellm_proxy/test_skills_ownership.py | 6 +----- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/litellm/llms/litellm_proxy/skills/handler.py b/litellm/llms/litellm_proxy/skills/handler.py index 1129af64e1..48c02660a0 100644 --- a/litellm/llms/litellm_proxy/skills/handler.py +++ b/litellm/llms/litellm_proxy/skills/handler.py @@ -137,12 +137,11 @@ class LiteLLMSkillsHandler: # Caller has no identity scope (no user_id / team_id / org_id / # api_key / token). Stamping a placeholder would let any two # identity-less callers see each other's skills via the shared - # owner — the cross-tenant primitive we avoid. - from fastapi import HTTPException - - raise HTTPException( - status_code=403, - detail="Unable to record skill ownership: caller has no identity scope.", + # owner — the cross-tenant primitive we avoid. ValueError keeps + # this module FastAPI-free per the project layering rule + # (litellm_proxy provider integrations live outside proxy/). + raise ValueError( + "Unable to record skill ownership: caller has no identity scope." ) skill_data: Dict[str, Any] = { diff --git a/litellm/proxy/auth/auth_utils.py b/litellm/proxy/auth/auth_utils.py index 9a6fc95f14..917d3d9922 100644 --- a/litellm/proxy/auth/auth_utils.py +++ b/litellm/proxy/auth/auth_utils.py @@ -224,14 +224,23 @@ def _build_banned_observability_params() -> FrozenSet[str]: the extras the canonical allowlist hasn't caught up to yet. New integrations added to the canonical allowlist are banned by default, which is the safe failure mode. + + ``_request_blocked_callback_params`` (e.g. ``gcs_bucket_name``, + ``gcs_path_service_account``) is the GCS-logging-specific deny list + that lives alongside the allowlist; fold it in here so a single + declaration of "this field must not be caller-supplied" covers both + the request-body bouncer and the dynamic callback initializer. """ from litellm.litellm_core_utils.initialize_dynamic_callback_params import ( + _request_blocked_callback_params, _supported_callback_params, ) return ( - frozenset(_supported_callback_params) - _SAFE_CLIENT_CALLBACK_PARAMS - ) | _EXTRA_BANNED_OBSERVABILITY_PARAMS + (frozenset(_supported_callback_params) - _SAFE_CLIENT_CALLBACK_PARAMS) + | _EXTRA_BANNED_OBSERVABILITY_PARAMS + | frozenset(_request_blocked_callback_params) + ) _BANNED_REQUEST_BODY_PARAMS: Tuple[str, ...] = ( diff --git a/tests/test_litellm/llms/litellm_proxy/test_skills_ownership.py b/tests/test_litellm/llms/litellm_proxy/test_skills_ownership.py index 8b189be492..c6dd6c4e33 100644 --- a/tests/test_litellm/llms/litellm_proxy/test_skills_ownership.py +++ b/tests/test_litellm/llms/litellm_proxy/test_skills_ownership.py @@ -265,8 +265,6 @@ async def test_should_reject_skill_create_for_identityless_proxy_auth(monkeypatc """Identity-less callers cannot create skills — stamping a shared sentinel as ``created_by`` would let any two such callers see each other's skills via the resulting shared owner scope.""" - from fastapi import HTTPException - table = AsyncMock() prisma_client = type( "Prisma", (), {"db": type("DB", (), {"litellm_skillstable": table})()} @@ -279,13 +277,11 @@ async def test_should_reject_skill_create_for_identityless_proxy_auth(monkeypatc auth = UserAPIKeyAuth() - with pytest.raises(HTTPException) as exc: + with pytest.raises(ValueError, match="identity scope"): await LiteLLMSkillsHandler.create_skill( data=NewSkillRequest(display_title="skill"), user_api_key_dict=auth, ) - assert exc.value.status_code == 403 - assert "identity scope" in str(exc.value.detail) table.create.assert_not_awaited()