From e4741fbb2b985df2155e5d97061cc5c33a33d66e Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Thu, 30 Apr 2026 18:53:44 -0700 Subject: [PATCH] fix(proxy): add legacy skill ownership opt-out --- litellm/llms/litellm_proxy/skills/handler.py | 41 +++++++++- .../litellm_proxy/test_skills_ownership.py | 78 +++++++++++++++++++ 2 files changed, 116 insertions(+), 3 deletions(-) diff --git a/litellm/llms/litellm_proxy/skills/handler.py b/litellm/llms/litellm_proxy/skills/handler.py index ddd5ab78c9d..7093874acd9 100644 --- a/litellm/llms/litellm_proxy/skills/handler.py +++ b/litellm/llms/litellm_proxy/skills/handler.py @@ -5,6 +5,7 @@ This module contains the actual database operations for skills CRUD. Used by the transformation layer and skills injection hook. """ +import os import uuid from typing import Any, Dict, List, Optional @@ -17,6 +18,32 @@ from litellm.proxy.common_utils.resource_ownership import ( user_can_access_resource_owner, ) +ALLOW_UNOWNED_SKILL_ACCESS_ENV = "LITELLM_ALLOW_UNOWNED_SKILL_ACCESS" + + +def _allow_unowned_skill_access() -> bool: + return os.getenv(ALLOW_UNOWNED_SKILL_ACCESS_ENV, "").lower() in { + "1", + "true", + "yes", + } + + +def _user_can_access_skill_owner( + owner: Optional[str], + user_api_key_dict: Optional[UserAPIKeyAuth], +) -> bool: + if owner is None and user_api_key_dict is not None: + if is_proxy_admin(user_api_key_dict): + return True + if _allow_unowned_skill_access(): + verbose_logger.warning( + "Allowing unowned skill access because %s is enabled", + ALLOW_UNOWNED_SKILL_ACCESS_ENV, + ) + return True + return user_can_access_resource_owner(owner, user_api_key_dict) + def _prisma_skill_to_litellm(prisma_skill) -> LiteLLM_SkillsTable: """ @@ -146,7 +173,15 @@ class LiteLLMSkillsHandler: owner_scopes = get_resource_owner_scopes(user_api_key_dict) if not owner_scopes: return [] - find_many_kwargs["where"] = {"created_by": {"in": owner_scopes}} + if _allow_unowned_skill_access(): + find_many_kwargs["where"] = { + "OR": [ + {"created_by": {"in": owner_scopes}}, + {"created_by": None}, + ] + } + else: + find_many_kwargs["where"] = {"created_by": {"in": owner_scopes}} skills = await prisma_client.db.litellm_skillstable.find_many( **find_many_kwargs @@ -182,7 +217,7 @@ class LiteLLMSkillsHandler: if skill is None: raise ValueError(f"Skill not found: {skill_id}") - if not user_can_access_resource_owner( + if not _user_can_access_skill_owner( getattr(skill, "created_by", None), user_api_key_dict ): raise ValueError(f"Skill not found: {skill_id}") @@ -218,7 +253,7 @@ class LiteLLMSkillsHandler: if skill is None: raise ValueError(f"Skill not found: {skill_id}") - if not user_can_access_resource_owner( + if not _user_can_access_skill_owner( getattr(skill, "created_by", None), user_api_key_dict ): raise ValueError(f"Skill not found: {skill_id}") 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 d5151b47a6f..fde1806f92c 100644 --- a/tests/test_litellm/llms/litellm_proxy/test_skills_ownership.py +++ b/tests/test_litellm/llms/litellm_proxy/test_skills_ownership.py @@ -3,9 +3,15 @@ from unittest.mock import AsyncMock import pytest from litellm.llms.litellm_proxy.skills.handler import LiteLLMSkillsHandler +from litellm.llms.litellm_proxy.skills import handler as skills_handler from litellm.proxy._types import LiteLLM_SkillsTable, NewSkillRequest, UserAPIKeyAuth +@pytest.fixture(autouse=True) +def clear_skill_ownership_env(monkeypatch): + monkeypatch.delenv(skills_handler.ALLOW_UNOWNED_SKILL_ACCESS_ENV, raising=False) + + def _skill(skill_id: str, created_by: str | None) -> LiteLLM_SkillsTable: return LiteLLM_SkillsTable( skill_id=skill_id, @@ -87,6 +93,78 @@ async def test_should_hide_skill_from_different_owner(monkeypatch): ) +@pytest.mark.asyncio +async def test_should_hide_unowned_skill_by_default(monkeypatch): + table = AsyncMock() + table.find_unique.return_value = _skill("litellm_skill_unowned", None) + prisma_client = type( + "Prisma", (), {"db": type("DB", (), {"litellm_skillstable": table})()} + )() + monkeypatch.setattr( + LiteLLMSkillsHandler, + "_get_prisma_client", + AsyncMock(return_value=prisma_client), + ) + + auth = UserAPIKeyAuth(user_id="user-1") + + with pytest.raises(ValueError, match="Skill not found"): + await LiteLLMSkillsHandler.get_skill( + "litellm_skill_unowned", + user_api_key_dict=auth, + ) + + +@pytest.mark.asyncio +async def test_should_allow_unowned_skill_when_enabled(monkeypatch): + table = AsyncMock() + table.find_unique.return_value = _skill("litellm_skill_unowned", None) + prisma_client = type( + "Prisma", (), {"db": type("DB", (), {"litellm_skillstable": table})()} + )() + monkeypatch.setattr( + LiteLLMSkillsHandler, + "_get_prisma_client", + AsyncMock(return_value=prisma_client), + ) + monkeypatch.setenv(skills_handler.ALLOW_UNOWNED_SKILL_ACCESS_ENV, "true") + + auth = UserAPIKeyAuth(user_id="user-1") + + skill = await LiteLLMSkillsHandler.get_skill( + "litellm_skill_unowned", + user_api_key_dict=auth, + ) + + assert skill.skill_id == "litellm_skill_unowned" + + +@pytest.mark.asyncio +async def test_should_include_unowned_skills_in_list_when_enabled(monkeypatch): + table = AsyncMock() + table.find_many.return_value = [_skill("litellm_skill_unowned", None)] + prisma_client = type( + "Prisma", (), {"db": type("DB", (), {"litellm_skillstable": table})()} + )() + monkeypatch.setattr( + LiteLLMSkillsHandler, + "_get_prisma_client", + AsyncMock(return_value=prisma_client), + ) + monkeypatch.setenv(skills_handler.ALLOW_UNOWNED_SKILL_ACCESS_ENV, "true") + + auth = UserAPIKeyAuth(user_id="user-1") + + skills = await LiteLLMSkillsHandler.list_skills(user_api_key_dict=auth) + + assert [skill.skill_id for skill in skills] == ["litellm_skill_unowned"] + where = table.find_many.await_args.kwargs["where"] + assert where["OR"] == [ + {"created_by": {"in": ["user-1", "user:user-1"]}}, + {"created_by": None}, + ] + + @pytest.mark.asyncio async def test_should_scope_skill_injection_fetch_to_authenticated_user(monkeypatch): from litellm.proxy.hooks.litellm_skills.main import SkillsInjectionHook