scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-09-14 23:21:35 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 102

fix(claude-code): close 4 skill-marketplace authz and supply-chain gaps

Browse source 
Non-admin callers could self-assign allowed_skills on a personal key with
no team/org ceiling; a disabled marketplace's skills stayed reachable
through pre-existing allowed_skills grants; a marketplace re-sync could
silently repoint an already-published skill's source with no re-review;
and a marketplace/skill name collision could overwrite a different
marketplace's (or a hand-registered) plugin row.
This commit is contained in:
Krrish Dholakia 2026-07-11 17:41:08 -07:00
parent 74b7e47316
commit 30897fd1cb
8 changed files with 413 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

30
litellm/proxy/anthropic_endpoints/claude_code_endpoints/claude_code_marketplace.py
View file

@ -37,7 +37,10 @@ from litellm.proxy.auth.user_api_key_auth import (
google_ai_studio_api_key_header,
user_api_key_auth,
)
from litellm.repositories.table_repositories import ClaudeCodePluginRepository
from litellm.repositories.table_repositories import (
ClaudeCodePluginRepository,
SkillMarketplaceRepository,
)
from litellm.types.proxy.claude_code_endpoints import (
ListPluginsResponse,
PluginListItem,
@ -118,18 +121,31 @@ async def get_marketplace(
prisma_client = await _get_prisma_client()
allowed_skills: FrozenSet[str] = (
await get_allowed_skills(user_api_key_dict, prisma_client)
if user_api_key_dict is not None
else frozenset()
await get_allowed_skills(user_api_key_dict, prisma_client) if user_api_key_dict is not None else frozenset()
)
where = (
{"OR": [{"enabled": True}, {"name": {"in": list(allowed_skills)}}]}
if allowed_skills
else {"enabled": True}
{"OR": [{"enabled": True}, {"name": {"in": list(allowed_skills)}}]} if allowed_skills else {"enabled": True}
)
plugins = await ClaudeCodePluginRepository(prisma_client).table.find_many(where=where)
# A per-skill grant (allowed_skills) is a standing entry on a
# key/team/org's object_permission - it isn't cleared just because an
# admin later disables the marketplace that owned the skill. Without
# this, a disabled marketplace's skills stay reachable forever by
# anyone previously granted one by name. Plugins with no
# marketplace_id (hand-registered) are unaffected.
marketplace_ids = {p.marketplace_id for p in plugins if p.marketplace_id}
if marketplace_ids:
disabled_marketplace_ids = {
m.id
for m in await SkillMarketplaceRepository(prisma_client).table.find_many(
where={"id": {"in": list(marketplace_ids)}, "enabled": False}
)
}
if disabled_marketplace_ids:
plugins = [p for p in plugins if p.marketplace_id not in disabled_marketplace_ids]
plugin_list = []
for plugin in plugins:
try:

90
litellm/proxy/anthropic_endpoints/claude_code_endpoints/claude_code_marketplace_sync.py
View file

@ -564,10 +564,7 @@ async def _fetch_entries_for_docs(
) -> tuple[tuple[ResolvedPluginEntry, ...], int]:
"""Returns (successfully-fetched entries, count skipped after retries)."""
fetched = await asyncio.gather(
*(
_fetch_skill_entry(client, repo, branch, marketplace_name, doc, timeout=timeout)
for doc in docs
)
*(_fetch_skill_entry(client, repo, branch, marketplace_name, doc, timeout=timeout) for doc in docs)
)
entries = tuple(entry for entry in fetched if entry is not None)
return entries, len(docs) - len(entries)
@ -657,11 +654,59 @@ def _build_plugin_manifest_json(entry: ResolvedPluginEntry) -> str:
)
def _existing_source_changed(existing_manifest_json: str, entry: ResolvedPluginEntry) -> bool:
try:
existing_source = json.loads(existing_manifest_json).get("source")
except json.JSONDecodeError:
return True
return existing_source != entry.source.model_dump()
async def _upsert_single_plugin(
repository: ClaudeCodePluginRepository, marketplace_id: str, entry: ResolvedPluginEntry
) -> None:
) -> bool:
"""Upsert one synced entry. Returns False (and writes nothing) if it was
skipped because ``entry.stored_name`` collides with a row owned by a
different marketplace - see the collision-guard comment below."""
now = datetime.now(timezone.utc)
manifest_json = _build_plugin_manifest_json(entry)
existing = await repository.table.find_unique(where={"name": entry.stored_name})
# `name` is globally unique but namespacing skills as "{marketplace}--{skill}"
# is only a convention, not schema-enforced - a marketplace slug or skill
# name chosen (or supplied by a compromised upstream repo) to collide with
# another marketplace's, or a hand-registered plugin's, stored_name must
# not silently overwrite that other row. Refuse and leave it untouched.
if existing is not None and existing.marketplace_id != marketplace_id:
verbose_proxy_logger.warning(
"skill-marketplace-sync: %r already registered under a different "
"marketplace (marketplace_id=%r), refusing to overwrite from marketplace_id=%r",
entry.stored_name,
existing.marketplace_id,
marketplace_id,
)
return False
# A skill an admin has already reviewed and published (enabled=True) must
# not have its git source silently swapped by whoever controls the
# upstream marketplace repo on the next sync - that would let a
# compromised/malicious upstream repoint an already-trusted, publicly
# served skill without any re-review. Demote it back to unpublished so an
# admin has to look at it again before it's public with the new source.
update_data: dict[str, Any] = {
"description": entry.description,
"manifest_json": manifest_json,
"marketplace_id": marketplace_id,
"updated_at": now,
}
if existing is not None and existing.enabled and _existing_source_changed(existing.manifest_json, entry):
verbose_proxy_logger.warning(
"skill-marketplace-sync: %r changed source on re-sync, unpublishing pending admin re-review",
entry.stored_name,
)
update_data["enabled"] = False
await repository.table.upsert(
where={"name": entry.stored_name},
data={
@ -675,14 +720,10 @@ async def _upsert_single_plugin(
"created_at": now,
"updated_at": now,
},
"update": {
"description": entry.description,
"manifest_json": manifest_json,
"marketplace_id": marketplace_id,
"updated_at": now,
},
"update": update_data,
},
)
return True
# prisma_client has no importable type stubs in this codebase (generated at
@ -692,9 +733,12 @@ async def _upsert_plugin_entries(
prisma_client: Any, # noqa: ANN401 # see comment above
marketplace_id: str,
entries: tuple[ResolvedPluginEntry, ...],
) -> None:
) -> int:
"""Returns the number of entries skipped due to a stored_name collision
with a row owned by a different marketplace."""
repository = ClaudeCodePluginRepository(prisma_client)
await asyncio.gather(*(_upsert_single_plugin(repository, marketplace_id, entry) for entry in entries))
written = await asyncio.gather(*(_upsert_single_plugin(repository, marketplace_id, entry) for entry in entries))
return sum(1 for ok in written if not ok)
async def _soft_disable_stale_plugins(
@ -758,15 +802,23 @@ async def resolve_and_sync(
await _record_sync_failure(prisma_client, marketplace_row, str(exc))
return SyncResult(status="error", error=str(exc), plugin_count=0)
if skipped_count:
collision_skipped_count = await _upsert_plugin_entries(prisma_client, marketplace_row.id, entries)
total_skipped_count = skipped_count + collision_skipped_count
if total_skipped_count:
verbose_proxy_logger.warning(
"skill-marketplace-sync: %r imported %d skill(s), skipped %d after retries",
"skill-marketplace-sync: %r imported %d skill(s), skipped %d (%d after retries, %d name collisions)",
marketplace_row.name,
len(entries),
len(entries) - collision_skipped_count,
total_skipped_count,
skipped_count,
collision_skipped_count,
)
await _upsert_plugin_entries(prisma_client, marketplace_row.id, entries)
await _soft_disable_stale_plugins(prisma_client, marketplace_row.id, entries)
await _record_sync_success(prisma_client, marketplace_row, source_type, skipped_count)
return SyncResult(status="success", error=None, plugin_count=len(entries), skipped_count=skipped_count)
await _record_sync_success(prisma_client, marketplace_row, source_type, total_skipped_count)
return SyncResult(
status="success",
error=None,
plugin_count=len(entries) - collision_skipped_count,
skipped_count=total_skipped_count,
)

5
litellm/proxy/auth/route_checks.py
View file

@ -722,6 +722,11 @@ class RouteChecks:
The Claude Code CLI fetches this route without custom headers, so it
allows passing key=api_key in the query params (mirrors the Google
generateContent carve-out above).
Known tradeoff, shared with the Google carve-out: a key passed this
way can end up unredacted in web-server access logs. Recommend admins
use a scoped, rotatable key for marketplace URLs rather than the
master key.
"""
return route == "/claude-code/marketplace.json"

16
litellm/proxy/management_endpoints/key_management_endpoints.py
View file

@ -78,6 +78,7 @@ from litellm.proxy.management_helpers.object_permission_utils import (
_set_object_permission,
attach_object_permission_to_dict,
handle_update_object_permission_common,
validate_key_allowed_skills_against_team,
validate_key_mcp_servers_against_team,
validate_key_search_tools_against_team,
validate_key_vector_stores_against_team,
@ -978,6 +979,11 @@ async def _common_key_generation_helper(
team_obj=team_table,
is_proxy_admin=_is_proxy_admin_caller,
)
await validate_key_allowed_skills_against_team(
object_permission=data_json.get("object_permission"),
team_obj=team_table,
is_proxy_admin=_is_proxy_admin_caller,
)
# Merge default_key_generate_params.object_permission in *after* the team-scope
# checks above, so an admin-configured default (e.g. vector_stores, search_tools)
@ -2235,6 +2241,11 @@ async def _validate_mcp_servers_for_key_update(
team_obj=effective_team_obj,
is_proxy_admin=is_proxy_admin,
)
await validate_key_allowed_skills_against_team(
object_permission=object_permission_dict,
team_obj=effective_team_obj,
is_proxy_admin=is_proxy_admin,
)
return normalized_object_permission
@ -4752,6 +4763,11 @@ async def regenerate_key_fn(
team_obj=regenerate_team_table,
is_proxy_admin=_regen_is_proxy_admin,
)
await validate_key_allowed_skills_against_team(
object_permission=_regen_object_permission_dict,
team_obj=regenerate_team_table,
is_proxy_admin=_regen_is_proxy_admin,
)
verbose_proxy_logger.info(
"Key regeneration requested: key_alias=%s",

40
litellm/proxy/management_helpers/object_permission_utils.py
View file

@ -707,6 +707,46 @@ async def validate_key_vector_stores_against_team(
)
def _extract_requested_allowed_skills(
object_permission: Optional[ObjectPermissionDict],
) -> set[str]:
"""Return allowed_skills names from a key's object_permission dict."""
if not object_permission or not isinstance(object_permission, dict):
return set()
raw = object_permission.get("allowed_skills")
if isinstance(raw, list):
return {str(x) for x in raw if x}
return set()
async def validate_key_allowed_skills_against_team(
object_permission: Optional[ObjectPermissionDict],
team_obj: Optional["LiteLLM_TeamTableCachedObj"],
is_proxy_admin: bool = False,
) -> None:
"""
Reject allowed_skills requested on a personal (no team) key by a non-admin
caller. Claude Code skill access is granted at use-time from the key's
object_permission.allowed_skills list, so the assignment is the
authorization boundary. Team keys and proxy admins are unaffected.
"""
requested = _extract_requested_allowed_skills(object_permission)
if not requested:
return
if team_obj is not None or is_proxy_admin:
return
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail={
"error": (
"Key is not in a team. Skills cannot be assigned to "
"personal keys by non-admin callers. Disallowed skills: "
f"{sorted(requested)}."
)
},
)
def _extract_requested_search_tools(
object_permission: Optional[ObjectPermissionDict],
) -> list[str]:

95
tests/pass_through_unit_tests/test_claude_code_marketplace.py
View file

@ -20,7 +20,6 @@ sys.path.insert(0, os.path.abspath("../.."))
import litellm
from litellm.proxy._types import UserAPIKeyAuth
from litellm.proxy.proxy_server import LitellmUserRoles
from litellm.caching.caching import DualCache
from litellm.types.proxy.claude_code_endpoints import RegisterPluginRequest
# Import the functions we're testing
@ -34,7 +33,14 @@ class MockPluginRecord:
"""Mock plugin record that mimics Prisma model behavior."""
def __init__(
self, name, version, description, manifest_json, enabled=True, created_by=None
self,
name,
version,
description,
manifest_json,
enabled=True,
created_by=None,
marketplace_id=None,
):
self.id = f"plugin-{name}-{int(time.time())}"
self.name = name
@ -46,6 +52,16 @@ class MockPluginRecord:
self.created_at = datetime.now(timezone.utc)
self.updated_at = datetime.now(timezone.utc)
self.created_by = created_by
self.marketplace_id = marketplace_id
class MockMarketplaceRecord:
"""Mock LiteLLM_SkillMarketplaceTable record."""
def __init__(self, id, name, enabled=True):
self.id = id
self.name = name
self.enabled = enabled
@pytest.fixture
@ -97,6 +113,7 @@ def mock_prisma_client():
manifest_json=manifest,
enabled=data.get("enabled", True),
created_by=data.get("created_by"),
marketplace_id=data.get("marketplace_id"),
)
plugins_store[plugin_name] = plugin
return plugin
@ -139,6 +156,25 @@ def mock_prisma_client():
mock_table.delete = AsyncMock(side_effect=delete)
mock_client.db.litellm_claudecodeplugintable = mock_table
# In-memory storage for marketplace rows, keyed by id.
marketplaces_store = {}
mock_marketplace_table = MagicMock()
async def marketplace_find_many(where=None):
rows = list(marketplaces_store.values())
if not where:
return rows
if "id" in where and "in" in where["id"]:
rows = [r for r in rows if r.id in where["id"]["in"]]
if "enabled" in where:
rows = [r for r in rows if r.enabled == where["enabled"]]
return rows
mock_marketplace_table.find_many = AsyncMock(side_effect=marketplace_find_many)
mock_client.db.litellm_skillmarketplacetable = mock_marketplace_table
mock_client._marketplaces_store = marketplaces_store
mock_client.connect = AsyncMock(side_effect=connect)
# Store plugins_store on the mock for cleanup if needed
@ -365,6 +401,61 @@ async def test_get_marketplace_with_key_unlocks_allowed_imported_skill(
)
@pytest.mark.asyncio
async def test_get_marketplace_hides_skill_from_disabled_marketplace_even_with_grant(
mock_prisma_client,
):
"""Regression test: allowed_skills is a standing grant on a key/team/org's
object_permission - it doesn't get cleared just because an admin later
disables the marketplace that owned the skill (DELETE
/claude-code/marketplaces/{name}, which cascades plugin.enabled=False but
leaves any pre-existing allowed_skills grants untouched). Without
filtering on the owning marketplace's own enabled state, a previously
granted key would keep seeing a skill from a marketplace an admin
explicitly shut off."""
from litellm.models.object_permission import LiteLLM_ObjectPermissionTable
setattr(litellm.proxy.proxy_server, "prisma_client", mock_prisma_client)
setattr(litellm.proxy.proxy_server, "master_key", "sk-1234")
await litellm.proxy.proxy_server.prisma_client.connect()
marketplace_id = f"marketplace-{int(time.time())}"
mock_prisma_client._marketplaces_store[marketplace_id] = MockMarketplaceRecord(
id=marketplace_id, name="untrusted-marketplace", enabled=False
)
skill_name = f"untrusted-marketplace--skill-{int(time.time())}"
await mock_prisma_client.db.litellm_claudecodeplugintable.create(
data={
"name": skill_name,
"version": "1.0.0",
"description": "Skill owned by a since-disabled marketplace",
"manifest_json": json.dumps(
{"source": {"source": "github", "repo": "org/untrusted-skill"}}
),
"enabled": True, # cascade-disable races aside, this asserts the belt-and-suspenders check too
"marketplace_id": marketplace_id,
}
)
granted_key = UserAPIKeyAuth(
user_role=LitellmUserRoles.INTERNAL_USER,
api_key="sk-granted",
user_id="granted-user",
object_permission=LiteLLM_ObjectPermissionTable(
object_permission_id="perm-2", allowed_skills=[skill_name]
),
)
response = await get_marketplace(user_api_key_dict=granted_key)
body = json.loads(response.body.decode())
assert skill_name not in {p["name"] for p in body["plugins"]}
# Cleanup
await mock_prisma_client.db.litellm_claudecodeplugintable.delete(where={"name": skill_name})
del mock_prisma_client._marketplaces_store[marketplace_id]
@pytest.mark.asyncio
async def test_register_plugin_git_subdir(mock_prisma_client):
"""Test registering a plugin with git-subdir source type."""

112
tests/test_litellm/proxy/anthropic_endpoints/test_claude_code_marketplace_sync.py
View file

@ -636,3 +636,115 @@ async def test_resolve_and_sync_soft_disables_stale_plugin(monkeypatch):
refreshed_stale = await client.db.litellm_claudecodeplugintable.find_unique(where={"name": stale_name})
assert refreshed_stale is not None
assert refreshed_stale.enabled is False
@pytest.mark.asyncio
async def test_resolve_and_sync_refuses_to_overwrite_row_owned_by_another_marketplace(monkeypatch):
"""Regression test: `name` is only conventionally namespaced as
"{marketplace}--{skill}", not schema-enforced, so a marketplace slug or
skill name that collides with an existing row owned by a *different*
marketplace (or a hand-registered plugin with no marketplace_id) must be
skipped, never silently overwritten."""
client = _make_fake_prisma_client()
colliding_name = "anthropic-agent-skills--claude-api"
hand_registered = await client.db.litellm_claudecodeplugintable.create(
data={
"name": colliding_name,
"description": "trusted, hand-registered plugin",
"manifest_json": json.dumps({"source": {"source": "github", "repo": "trusted/repo"}}),
"files_json": "{}",
"enabled": True,
"marketplace_id": None,
"created_at": datetime(2023, 1, 1),
"updated_at": datetime(2023, 1, 1),
}
)
marketplace = await _create_marketplace(client, name="anthropic-agent-skills", source_ref="anthropics/skills")
async def _get(http_client, url, **kwargs):
return httpx.Response(200, json=_ANTHROPIC_SKILLS_MANIFEST)
monkeypatch.setattr(sync_module, "async_safe_get", _get)
result = await resolve_and_sync(client, marketplace)
assert result.status == "success"
assert result.plugin_count == 2 # document-skills, example-skills - claude-api collided and was skipped
assert result.skipped_count == 1
untouched = await client.db.litellm_claudecodeplugintable.find_unique(where={"name": colliding_name})
assert untouched.marketplace_id is None
assert untouched.enabled is True
assert json.loads(untouched.manifest_json)["source"] == {"source": "github", "repo": "trusted/repo"}
assert untouched.id == hand_registered.id
@pytest.mark.asyncio
async def test_resolve_and_sync_unpublishes_skill_whose_source_changed(monkeypatch):
"""Regression test: an already-public (enabled=True) skill must not have
its git source silently swapped by a re-sync of the marketplace that owns
it - that would let a compromised/malicious upstream repoint an
already-trusted skill with no admin re-review. The sync should demote it
back to enabled=False instead of overwriting the source in place."""
client = _make_fake_prisma_client()
marketplace = await _create_marketplace(client, name="anthropic-agent-skills", source_ref="anthropics/skills")
async def _get(http_client, url, **kwargs):
return httpx.Response(200, json=_ANTHROPIC_SKILLS_MANIFEST)
monkeypatch.setattr(sync_module, "async_safe_get", _get)
await resolve_and_sync(client, marketplace)
published_name = "anthropic-agent-skills--claude-api"
published = await client.db.litellm_claudecodeplugintable.find_unique(where={"name": published_name})
published.enabled = True # simulate an admin having reviewed and published it
original_source = json.loads(published.manifest_json)["source"]
# A repointed `source` (repo root -> a subdirectory) is exactly what an
# upstream marketplace repo owner controls and could change unilaterally.
changed_manifest = {
**_ANTHROPIC_SKILLS_MANIFEST,
"plugins": [
p if p["name"] != "claude-api" else {**p, "source": "./skills/claude-api"}
for p in _ANTHROPIC_SKILLS_MANIFEST["plugins"]
],
}
async def _get_changed(http_client, url, **kwargs):
return httpx.Response(200, json=changed_manifest)
monkeypatch.setattr(sync_module, "async_safe_get", _get_changed)
result = await resolve_and_sync(client, marketplace)
assert result.status == "success"
refreshed = await client.db.litellm_claudecodeplugintable.find_unique(where={"name": published_name})
assert refreshed.enabled is False
new_source = json.loads(refreshed.manifest_json)["source"]
assert new_source != original_source
assert new_source["path"] == "skills/claude-api"
@pytest.mark.asyncio
async def test_resolve_and_sync_leaves_published_skill_enabled_when_source_unchanged(monkeypatch):
"""A re-sync that resolves to the exact same source must not touch an
already-published skill's enabled state."""
client = _make_fake_prisma_client()
marketplace = await _create_marketplace(client, name="anthropic-agent-skills", source_ref="anthropics/skills")
async def _get(http_client, url, **kwargs):
return httpx.Response(200, json=_ANTHROPIC_SKILLS_MANIFEST)
monkeypatch.setattr(sync_module, "async_safe_get", _get)
await resolve_and_sync(client, marketplace)
published_name = "anthropic-agent-skills--claude-api"
published = await client.db.litellm_claudecodeplugintable.find_unique(where={"name": published_name})
published.enabled = True
await resolve_and_sync(client, marketplace)
refreshed = await client.db.litellm_claudecodeplugintable.find_unique(where={"name": published_name})
assert refreshed.enabled is True

53
tests/test_litellm/proxy/management_helpers/test_object_permission_utils.py
View file

@ -22,6 +22,7 @@ from litellm.proxy.management_helpers.object_permission_utils import (
_rewrite_object_permission_mcp_servers,
_set_object_permission,
enforce_all_proxy_mcp_servers_grant_is_admin_only,
validate_key_allowed_skills_against_team,
validate_key_mcp_servers_against_team,
validate_key_search_tools_against_team,
validate_key_vector_stores_against_team,
@ -1216,6 +1217,58 @@ async def test_empty_object_permission_passes_for_personal_non_admin():
)
@pytest.mark.asyncio
async def test_personal_non_admin_cannot_assign_allowed_skills():
"""
Regression test: a non-admin caller with no team must not be able to
self-assign Claude Code skill access on their own key - allowed_skills
is the authorization boundary get_allowed_skills() reads from directly,
with no team/org ceiling to fall back on for a personal key.
"""
with pytest.raises(HTTPException) as exc:
await validate_key_allowed_skills_against_team(
object_permission={"allowed_skills": ["anthropic-agent-skills--private-skill"]},
team_obj=None,
is_proxy_admin=False,
)
assert exc.value.status_code == 403
assert "anthropic-agent-skills--private-skill" in str(exc.value.detail)
@pytest.mark.asyncio
async def test_personal_admin_can_assign_allowed_skills():
await validate_key_allowed_skills_against_team(
object_permission={"allowed_skills": ["anthropic-agent-skills--private-skill"]},
team_obj=None,
is_proxy_admin=True,
)
@pytest.mark.asyncio
async def test_team_key_allowed_skills_unrestricted_at_create():
"""Team-scoped keys retain their existing trust model at create time."""
team_obj = _make_team_obj_search()
await validate_key_allowed_skills_against_team(
object_permission={"allowed_skills": ["anthropic-agent-skills--anything"]},
team_obj=team_obj,
is_proxy_admin=False,
)
@pytest.mark.asyncio
async def test_empty_allowed_skills_passes_for_personal_non_admin():
await validate_key_allowed_skills_against_team(
object_permission=None,
team_obj=None,
is_proxy_admin=False,
)
await validate_key_allowed_skills_against_team(
object_permission={"allowed_skills": []},
team_obj=None,
is_proxy_admin=False,
)
def test_object_permission_dict_mirrors_pydantic_model():
"""ObjectPermissionDict must stay field-for-field aligned with
LiteLLM_ObjectPermissionBase. If a new field is added to the Pydantic