mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-07-22 07:15:10 +00:00
Authenticate skill registry downloads (#6600)
Registry downloads initialized PlusAPI without credentials, so uncached skills failed outside CLI-authenticated flows and were blocked entirely in non-interactive environments. Use CREWAI_USER_PAT first, then the platform integration token, then the saved login token, and pass CREWAI_ORGANIZATION_UUID. Remove the non-interactive cache-only restriction so runtime downloads work.
This commit is contained in:
@@ -12,7 +12,6 @@ import sys
|
||||
from crewai.skills import cache, events, registry
|
||||
from crewai.skills.cache import SkillCacheManager
|
||||
from crewai.skills.registry import (
|
||||
SkillNotCachedError,
|
||||
is_registry_ref,
|
||||
parse_registry_ref,
|
||||
resolve_registry_ref,
|
||||
@@ -27,7 +26,6 @@ sys.modules[__name__ + ".registry"] = registry
|
||||
|
||||
__all__ = [
|
||||
"SkillCacheManager",
|
||||
"SkillNotCachedError",
|
||||
"cache",
|
||||
"events",
|
||||
"is_registry_ref",
|
||||
|
||||
@@ -15,7 +15,6 @@ from crewai.skills.loader import (
|
||||
from crewai.skills.models import Skill, SkillFrontmatter
|
||||
from crewai.skills.parser import SkillParseError
|
||||
from crewai.skills.registry import (
|
||||
SkillNotCachedError,
|
||||
is_registry_ref,
|
||||
parse_registry_ref,
|
||||
resolve_registry_ref,
|
||||
@@ -26,7 +25,6 @@ __all__ = [
|
||||
"Skill",
|
||||
"SkillCacheManager",
|
||||
"SkillFrontmatter",
|
||||
"SkillNotCachedError",
|
||||
"SkillParseError",
|
||||
"activate_skill",
|
||||
"discover_skills",
|
||||
|
||||
@@ -7,8 +7,8 @@ via the CrewAI+ API with a global cache at ~/.crewai/skills/.
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
import os
|
||||
from pathlib import Path
|
||||
import sys
|
||||
from typing import Any
|
||||
|
||||
from crewai.skills.cache import SkillCacheManager
|
||||
@@ -17,17 +17,6 @@ from crewai.skills.cache import SkillCacheManager
|
||||
_logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class SkillNotCachedError(Exception):
|
||||
"""Raised when a registry skill is not cached and the environment is non-interactive."""
|
||||
|
||||
def __init__(self, ref: str) -> None:
|
||||
super().__init__(
|
||||
f"Skill {ref!r} is not cached locally. "
|
||||
f"Run `crewai skill install {ref}` to install it first."
|
||||
)
|
||||
self.ref = ref
|
||||
|
||||
|
||||
def is_registry_ref(value: Any) -> bool:
|
||||
"""Return True if *value* looks like a registry reference (@org/name)."""
|
||||
return isinstance(value, str) and value.startswith("@")
|
||||
@@ -68,17 +57,6 @@ def parse_registry_ref(ref: str) -> tuple[str, str]:
|
||||
return org, name
|
||||
|
||||
|
||||
def _is_noninteractive() -> bool:
|
||||
"""Return True in CI or explicitly non-interactive environments."""
|
||||
import os
|
||||
|
||||
return (
|
||||
os.environ.get("CI") == "1"
|
||||
or os.environ.get("CREWAI_NONINTERACTIVE") == "1"
|
||||
or not sys.stdin.isatty()
|
||||
)
|
||||
|
||||
|
||||
def resolve_registry_ref(
|
||||
ref: str,
|
||||
source: Any = None,
|
||||
@@ -88,7 +66,7 @@ def resolve_registry_ref(
|
||||
Resolution order:
|
||||
1. ./skills/{name}/ in the current working directory (project-local)
|
||||
2. ~/.crewai/skills/{org}/{name}/ (global cache)
|
||||
3. Download from registry (interactive only; raises SkillNotCachedError in CI)
|
||||
3. Download from registry
|
||||
|
||||
Args:
|
||||
ref: A registry reference, e.g. '@acme/my-skill'.
|
||||
@@ -96,9 +74,6 @@ def resolve_registry_ref(
|
||||
|
||||
Returns:
|
||||
A Skill loaded at INSTRUCTIONS disclosure level.
|
||||
|
||||
Raises:
|
||||
SkillNotCachedError: When not cached and running in non-interactive mode.
|
||||
"""
|
||||
from crewai.skills.loader import activate_skill
|
||||
from crewai.skills.parser import load_skill_metadata
|
||||
@@ -124,9 +99,6 @@ def resolve_registry_ref(
|
||||
"Failed to load cached skill at %s", cached_path, exc_info=True
|
||||
)
|
||||
|
||||
if _is_noninteractive():
|
||||
raise SkillNotCachedError(ref)
|
||||
|
||||
return download_skill(org, name, source=source)
|
||||
|
||||
|
||||
@@ -172,7 +144,15 @@ def download_skill(
|
||||
try:
|
||||
from crewai_core.plus_api import PlusAPI
|
||||
|
||||
api = PlusAPI()
|
||||
from crewai.auth.token import get_auth_token
|
||||
from crewai.context import get_platform_integration_token
|
||||
|
||||
api = PlusAPI(
|
||||
api_key=os.getenv("CREWAI_USER_PAT")
|
||||
or get_platform_integration_token()
|
||||
or get_auth_token(),
|
||||
organization_id=os.getenv("CREWAI_ORGANIZATION_UUID"),
|
||||
)
|
||||
response = api.get_skill(org, name)
|
||||
response.raise_for_status()
|
||||
data = response.json()
|
||||
|
||||
@@ -5,14 +5,12 @@ def test_experimental_namespace_reexports_public_names():
|
||||
from crewai.experimental import skills as experimental_skills
|
||||
from crewai.skills.cache import SkillCacheManager
|
||||
from crewai.skills.registry import (
|
||||
SkillNotCachedError,
|
||||
is_registry_ref,
|
||||
parse_registry_ref,
|
||||
resolve_registry_ref,
|
||||
)
|
||||
|
||||
assert experimental_skills.SkillCacheManager is SkillCacheManager
|
||||
assert experimental_skills.SkillNotCachedError is SkillNotCachedError
|
||||
assert experimental_skills.is_registry_ref is is_registry_ref
|
||||
assert experimental_skills.parse_registry_ref is parse_registry_ref
|
||||
assert experimental_skills.resolve_registry_ref is resolve_registry_ref
|
||||
|
||||
@@ -1,18 +1,40 @@
|
||||
"""Tests for SkillRegistry."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import base64
|
||||
from io import BytesIO
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, patch
|
||||
from zipfile import ZipFile
|
||||
|
||||
from crewai.context import platform_context
|
||||
from crewai.skills.cache import SkillCacheManager
|
||||
from crewai.skills.registry import (
|
||||
SkillNotCachedError,
|
||||
download_skill,
|
||||
is_registry_ref,
|
||||
parse_registry_ref,
|
||||
)
|
||||
import pytest
|
||||
|
||||
|
||||
def _skill_archive(name: str) -> bytes:
|
||||
archive = BytesIO()
|
||||
with ZipFile(archive, "w") as zip_file:
|
||||
zip_file.writestr(
|
||||
"SKILL.md",
|
||||
f"---\nname: {name}\ndescription: Test skill.\n---\n\nInstructions.",
|
||||
)
|
||||
return archive.getvalue()
|
||||
|
||||
|
||||
def _mock_skill_response(name: str) -> MagicMock:
|
||||
response = MagicMock()
|
||||
response.json.return_value = {
|
||||
"latest_version": "1.0.0",
|
||||
"file": base64.b64encode(_skill_archive(name)).decode(),
|
||||
}
|
||||
return response
|
||||
|
||||
|
||||
class TestIsRegistryRef:
|
||||
def test_at_prefixed(self) -> None:
|
||||
assert is_registry_ref("@acme/my-skill") is True
|
||||
@@ -54,10 +76,7 @@ class TestParseRegistryRef:
|
||||
|
||||
|
||||
class TestResolveRegistryRef:
|
||||
"""Test resolution order and CI mode behaviour."""
|
||||
|
||||
def _make_skill_dir(self, base: Path, name: str) -> Path:
|
||||
"""Write a minimal SKILL.md into base/name/."""
|
||||
skill_dir = base / name
|
||||
skill_dir.mkdir(parents=True)
|
||||
(skill_dir / "SKILL.md").write_text(
|
||||
@@ -65,42 +84,58 @@ class TestResolveRegistryRef:
|
||||
)
|
||||
return skill_dir
|
||||
|
||||
def test_resolves_project_local(self, tmp_path: Path) -> None:
|
||||
"""Local ./skills/{name}/ takes priority over cache."""
|
||||
def test_prefers_project_local_skill_over_cached_skill(
|
||||
self, tmp_path: Path
|
||||
) -> None:
|
||||
skills_dir = tmp_path / "skills"
|
||||
skills_dir.mkdir()
|
||||
self._make_skill_dir(skills_dir, "my-skill")
|
||||
|
||||
cache_dir = tmp_path / "cache" / "acme" / "my-skill"
|
||||
cache_dir.mkdir(parents=True)
|
||||
(cache_dir / "SKILL.md").write_text(
|
||||
"---\nname: cached-skill\ndescription: Cached.\n---\n\nCached instructions."
|
||||
)
|
||||
mock_cache = MagicMock()
|
||||
mock_cache.get_cached_path.return_value = None
|
||||
mock_cache.get_cached_path.return_value = cache_dir
|
||||
|
||||
with (
|
||||
patch("crewai.skills.registry._is_noninteractive", return_value=False),
|
||||
patch.object(Path, "cwd", return_value=tmp_path),
|
||||
patch("crewai.skills.registry.SkillCacheManager", return_value=mock_cache),
|
||||
):
|
||||
from crewai.skills.registry import resolve_registry_ref
|
||||
|
||||
skill = resolve_registry_ref("@acme/my-skill")
|
||||
|
||||
assert skill.name == "my-skill"
|
||||
|
||||
def test_raises_in_ci_when_not_cached(self, tmp_path: Path) -> None:
|
||||
"""In CI mode, raise SkillNotCachedError if no local or cached copy."""
|
||||
mock_cache = MagicMock()
|
||||
mock_cache.get_cached_path.return_value = None
|
||||
def test_downloads_and_caches_uncached_skill_in_noninteractive_environment(
|
||||
self, tmp_path: Path
|
||||
) -> None:
|
||||
cache = SkillCacheManager(cache_root=tmp_path / "cache")
|
||||
api = MagicMock()
|
||||
api.get_skill.return_value = _mock_skill_response("ghost-skill")
|
||||
|
||||
with (
|
||||
patch("crewai.skills.registry._is_noninteractive", return_value=True),
|
||||
patch.dict("os.environ", {"CI": "1", "CREWAI_NONINTERACTIVE": "1"}),
|
||||
patch.object(Path, "cwd", return_value=tmp_path),
|
||||
patch("crewai.skills.registry.SkillCacheManager", return_value=mock_cache),
|
||||
patch("crewai.auth.token.get_auth_token", return_value="saved-login"),
|
||||
patch("crewai.skills.registry.SkillCacheManager", return_value=cache),
|
||||
patch("crewai_core.plus_api.PlusAPI", return_value=api),
|
||||
):
|
||||
from crewai.skills.registry import resolve_registry_ref
|
||||
with pytest.raises(SkillNotCachedError) as exc_info:
|
||||
resolve_registry_ref("@acme/ghost-skill")
|
||||
assert "@acme/ghost-skill" in str(exc_info.value)
|
||||
|
||||
def test_resolves_from_cache(self, tmp_path: Path) -> None:
|
||||
"""Falls back to global cache when no project-local skill exists."""
|
||||
skill = resolve_registry_ref("@acme/ghost-skill")
|
||||
|
||||
assert skill.name == "ghost-skill"
|
||||
api.get_skill.assert_called_once_with("acme", "ghost-skill")
|
||||
assert cache.get_cached_path("acme", "ghost-skill") == (
|
||||
tmp_path / "cache" / "acme" / "ghost-skill"
|
||||
)
|
||||
|
||||
def test_resolves_cached_skill_when_project_local_skill_is_missing(
|
||||
self, tmp_path: Path
|
||||
) -> None:
|
||||
cache_dir = tmp_path / "acme" / "cached-skill"
|
||||
cache_dir.mkdir(parents=True)
|
||||
(cache_dir / "SKILL.md").write_text(
|
||||
@@ -110,18 +145,89 @@ class TestResolveRegistryRef:
|
||||
mock_cache = MagicMock()
|
||||
mock_cache.get_cached_path.return_value = cache_dir
|
||||
|
||||
# tmp_path has no ./skills/ directory
|
||||
with (
|
||||
patch("crewai.skills.registry._is_noninteractive", return_value=False),
|
||||
patch.object(Path, "cwd", return_value=tmp_path),
|
||||
patch("crewai.skills.registry.SkillCacheManager", return_value=mock_cache),
|
||||
):
|
||||
from crewai.skills.registry import resolve_registry_ref
|
||||
|
||||
skill = resolve_registry_ref("@acme/cached-skill")
|
||||
|
||||
assert skill.name == "cached-skill"
|
||||
|
||||
def test_skill_not_cached_error_contains_ref(self) -> None:
|
||||
err = SkillNotCachedError("@foo/bar")
|
||||
assert "@foo/bar" in str(err)
|
||||
assert err.ref == "@foo/bar"
|
||||
|
||||
class TestDownloadSkillAuthentication:
|
||||
def test_user_pat_takes_precedence_over_platform_token(
|
||||
self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
cache = SkillCacheManager(cache_root=tmp_path / "cache")
|
||||
api = MagicMock()
|
||||
api.get_skill.return_value = _mock_skill_response("my-skill")
|
||||
monkeypatch.setenv("CREWAI_USER_PAT", "user-pat")
|
||||
monkeypatch.delenv("CREWAI_ORGANIZATION_UUID", raising=False)
|
||||
|
||||
with (
|
||||
platform_context("platform-token"),
|
||||
patch("crewai.skills.registry.SkillCacheManager", return_value=cache),
|
||||
patch("crewai_core.plus_api.PlusAPI", return_value=api) as plus_api,
|
||||
):
|
||||
download_skill("acme", "my-skill")
|
||||
|
||||
plus_api.assert_called_once_with(api_key="user-pat", organization_id=None)
|
||||
|
||||
def test_uses_platform_token_when_user_pat_is_missing(
|
||||
self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
cache = SkillCacheManager(cache_root=tmp_path / "cache")
|
||||
api = MagicMock()
|
||||
api.get_skill.return_value = _mock_skill_response("my-skill")
|
||||
monkeypatch.delenv("CREWAI_USER_PAT", raising=False)
|
||||
monkeypatch.delenv("CREWAI_ORGANIZATION_UUID", raising=False)
|
||||
|
||||
with (
|
||||
platform_context("platform-token"),
|
||||
patch("crewai.skills.registry.SkillCacheManager", return_value=cache),
|
||||
patch("crewai_core.plus_api.PlusAPI", return_value=api) as plus_api,
|
||||
):
|
||||
download_skill("acme", "my-skill")
|
||||
|
||||
plus_api.assert_called_once_with(api_key="platform-token", organization_id=None)
|
||||
|
||||
def test_uses_user_pat_and_organization_from_environment(
|
||||
self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
cache = SkillCacheManager(cache_root=tmp_path / "cache")
|
||||
api = MagicMock()
|
||||
api.get_skill.return_value = _mock_skill_response("my-skill")
|
||||
monkeypatch.delenv("CREWAI_PLATFORM_INTEGRATION_TOKEN", raising=False)
|
||||
monkeypatch.setenv("CREWAI_USER_PAT", "user-pat")
|
||||
monkeypatch.setenv("CREWAI_ORGANIZATION_UUID", "organization-uuid")
|
||||
|
||||
with (
|
||||
patch("crewai.skills.registry.SkillCacheManager", return_value=cache),
|
||||
patch("crewai_core.plus_api.PlusAPI", return_value=api) as plus_api,
|
||||
):
|
||||
download_skill("acme", "my-skill")
|
||||
|
||||
plus_api.assert_called_once_with(
|
||||
api_key="user-pat", organization_id="organization-uuid"
|
||||
)
|
||||
|
||||
def test_uses_saved_cli_login_when_runtime_tokens_are_missing(
|
||||
self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
cache = SkillCacheManager(cache_root=tmp_path / "cache")
|
||||
api = MagicMock()
|
||||
api.get_skill.return_value = _mock_skill_response("my-skill")
|
||||
monkeypatch.delenv("CREWAI_USER_PAT", raising=False)
|
||||
monkeypatch.delenv("CREWAI_PLATFORM_INTEGRATION_TOKEN", raising=False)
|
||||
monkeypatch.delenv("CREWAI_ORGANIZATION_UUID", raising=False)
|
||||
|
||||
with (
|
||||
patch("crewai.auth.token.get_auth_token", return_value="saved-login"),
|
||||
patch("crewai.skills.registry.SkillCacheManager", return_value=cache),
|
||||
patch("crewai_core.plus_api.PlusAPI", return_value=api) as plus_api,
|
||||
):
|
||||
download_skill("acme", "my-skill")
|
||||
|
||||
plus_api.assert_called_once_with(api_key="saved-login", organization_id=None)
|
||||
|
||||
Reference in New Issue
Block a user