mirror of
https://github.com/tiennm99/litellm.git
synced 2026-08-07 14:25:13 +00:00
fix: keep skills handler FastAPI-free; fold gcs deny list into the body bouncer
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.
This commit is contained in:
@@ -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] = {
|
||||
|
||||
@@ -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, ...] = (
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user