fix(proxy): add legacy skill ownership opt-out

This commit is contained in:
user 2026-04-30 18:53:44 -07:00
parent 0745b1872e
commit e4741fbb2b
2 changed files with 116 additions and 3 deletions

View file

@ -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}")

View file

@ -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