diff --git a/lib/cli/src/crewai_cli/skills/main.py b/lib/cli/src/crewai_cli/skills/main.py index a31599714..9aa6f6a31 100644 --- a/lib/cli/src/crewai_cli/skills/main.py +++ b/lib/cli/src/crewai_cli/skills/main.py @@ -6,10 +6,10 @@ import base64 import io import json import os +import tarfile from pathlib import Path import zipfile -import click from rich.console import Console from rich.table import Table @@ -271,7 +271,7 @@ class SkillCommand(BaseCommand, PlusAPIMixin): str(skill_dir), ) except (json.JSONDecodeError, KeyError): - pass + console.print(f"[yellow]Warning: skipping malformed cache entry at {meta_file}[/yellow]") console.print(table) @@ -281,12 +281,9 @@ class SkillCommand(BaseCommand, PlusAPIMixin): def _unpack_archive(self, archive_bytes: bytes, dest: Path) -> None: """Unpack a .tar.gz or .zip archive into dest.""" - import tarfile - # Try tar first, then zip try: - import io as _io - with tarfile.open(fileobj=_io.BytesIO(archive_bytes), mode="r:gz") as tf: + with tarfile.open(fileobj=io.BytesIO(archive_bytes), mode="r:gz") as tf: try: tf.extractall(dest, filter="data") except TypeError: @@ -297,7 +294,7 @@ class SkillCommand(BaseCommand, PlusAPIMixin): # Fallback: zip with zipfile.ZipFile(io.BytesIO(archive_bytes)) as zf: - zf.extractall(dest) + _safe_extract_zip(zf, dest) def _build_skill_zip(self) -> bytes: """Build an in-memory ZIP of SKILL.md + scripts/ + references/ + assets/.""" @@ -319,7 +316,7 @@ class SkillCommand(BaseCommand, PlusAPIMixin): if not match: raise ValueError("No YAML frontmatter block found") try: - import yaml # type: ignore[import-untyped] + import yaml return yaml.safe_load(match.group(1)) or {} except ImportError: # Minimal fallback: parse key: value lines @@ -339,12 +336,21 @@ class SkillCommand(BaseCommand, PlusAPIMixin): return None -def _safe_extractall(tf: "tarfile.TarFile", dest: Path) -> None: +def _safe_extractall(tf: tarfile.TarFile, dest: Path) -> None: """Path-traversal-safe extraction for Python < 3.12.""" - import tarfile dest_resolved = dest.resolve() for member in tf.getmembers(): member_path = (dest / member.name).resolve() - if not str(member_path).startswith(str(dest_resolved)): + if not member_path.is_relative_to(dest_resolved): raise ValueError(f"Blocked path traversal attempt: {member.name!r}") tf.extractall(dest) # noqa: S202 + + +def _safe_extract_zip(zf: zipfile.ZipFile, dest: Path) -> None: + """Path-traversal-safe ZIP extraction.""" + dest_resolved = dest.resolve() + for member in zf.namelist(): + member_path = (dest / member).resolve() + if not member_path.is_relative_to(dest_resolved): + raise ValueError(f"Blocked path traversal attempt: {member!r}") + zf.extractall(dest) # noqa: S202 diff --git a/lib/cli/tests/skills/test_main.py b/lib/cli/tests/skills/test_main.py index 376ac5608..e3b49da21 100644 --- a/lib/cli/tests/skills/test_main.py +++ b/lib/cli/tests/skills/test_main.py @@ -3,7 +3,6 @@ from __future__ import annotations import io -import json import os import tempfile import zipfile diff --git a/lib/crewai/src/crewai/agent/core.py b/lib/crewai/src/crewai/agent/core.py index c2c8848bc..1dc0ffc57 100644 --- a/lib/crewai/src/crewai/agent/core.py +++ b/lib/crewai/src/crewai/agent/core.py @@ -434,7 +434,7 @@ class Agent(BaseAgent): from crewai.crew import Crew if resolved_crew_skills is None: - crew_skills: list[Path | SkillModel] | None = ( + crew_skills: list[Path | SkillModel | str] | None = ( self.crew.skills if isinstance(self.crew, Crew) and isinstance(self.crew.skills, list) else None @@ -454,7 +454,7 @@ class Agent(BaseAgent): return seen: set[str] = set() - resolved: list[Path | SkillModel] = [] + resolved: list[Path | SkillModel | str] = [] items: list[Path | SkillModel | str] = list(self.skills) if self.skills else [] if crew_skills: diff --git a/lib/crewai/src/crewai/skills/cache.py b/lib/crewai/src/crewai/skills/cache.py index e3930e8fd..231ab93b1 100644 --- a/lib/crewai/src/crewai/skills/cache.py +++ b/lib/crewai/src/crewai/skills/cache.py @@ -7,11 +7,14 @@ One version is stored per skill (last install wins). from __future__ import annotations import json +import logging import tarfile from datetime import datetime, timezone from pathlib import Path from typing import TypedDict +_logger = logging.getLogger(__name__) + _CACHE_ROOT = Path.home() / ".crewai" / "skills" _META_FILENAME = ".crewai_meta.json" @@ -93,7 +96,7 @@ class SkillCacheManager: try: results.append(json.loads(meta_file.read_text())) except (json.JSONDecodeError, KeyError): - pass + _logger.debug("Skipping malformed cache entry: %s", meta_file, exc_info=True) return results def invalidate(self, org: str, name: str) -> bool: @@ -115,6 +118,6 @@ def _safe_extractall(tf: tarfile.TarFile, dest: Path) -> None: dest_resolved = dest.resolve() for member in tf.getmembers(): member_path = (dest / member.name).resolve() - if not str(member_path).startswith(str(dest_resolved)): + if not member_path.is_relative_to(dest_resolved): raise ValueError(f"Blocked path traversal attempt: {member.name!r}") tf.extractall(dest) # noqa: S202 diff --git a/lib/crewai/src/crewai/skills/registry.py b/lib/crewai/src/crewai/skills/registry.py index 20c4987a8..5daa23138 100644 --- a/lib/crewai/src/crewai/skills/registry.py +++ b/lib/crewai/src/crewai/skills/registry.py @@ -6,12 +6,15 @@ via the CrewAI+ API with a global cache at ~/.crewai/skills/. from __future__ import annotations +import logging import sys from pathlib import Path from typing import Any 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.""" @@ -19,7 +22,7 @@ class SkillNotCachedError(Exception): def __init__(self, ref: str) -> None: super().__init__( f"Skill {ref!r} is not cached locally. " - "Run `crewai skill install {ref}` to install it first." + f"Run `crewai skill install {ref}` to install it first." ) self.ref = ref @@ -99,7 +102,7 @@ def resolve_registry_ref( skill = load_skill_metadata(local_path) return activate_skill(skill, source=source) except Exception: - pass + _logger.debug("Failed to load local skill at %s", local_path, exc_info=True) # 2. Global cache cache = SkillCacheManager() @@ -109,7 +112,7 @@ def resolve_registry_ref( skill = load_skill_metadata(cached_path) return activate_skill(skill, source=source) except Exception: - pass + _logger.debug("Failed to load cached skill at %s", cached_path, exc_info=True) # 3. Download if _is_noninteractive(): @@ -139,16 +142,16 @@ def download_skill( ref = f"@{org}/{name}" try: - from crewai.events.crewai_event_bus import crewai_event_bus + from crewai.events.event_bus import crewai_event_bus from crewai.events.types.skill_events import SkillDownloadStartedEvent, SkillDownloadCompletedEvent _has_events = True except ImportError: _has_events = False if _has_events: - crewai_event_bus.emit( # type: ignore[possibly-undefined] + crewai_event_bus.emit( source, - event=SkillDownloadStartedEvent( # type: ignore[possibly-undefined] + event=SkillDownloadStartedEvent( registry_ref=ref, ), ) @@ -176,9 +179,9 @@ def download_skill( skill_dir = cache.store(org, name, version, archive_bytes) if _has_events: - crewai_event_bus.emit( # type: ignore[possibly-undefined] + crewai_event_bus.emit( source, - event=SkillDownloadCompletedEvent( # type: ignore[possibly-undefined] + event=SkillDownloadCompletedEvent( registry_ref=ref, version=version, cache_path=skill_dir, diff --git a/lib/crewai/tests/skills/test_cache.py b/lib/crewai/tests/skills/test_cache.py index bd86ce09c..37b3e9e30 100644 --- a/lib/crewai/tests/skills/test_cache.py +++ b/lib/crewai/tests/skills/test_cache.py @@ -8,8 +8,6 @@ import json import tarfile from pathlib import Path -import pytest - from crewai.skills.cache import SkillCacheManager diff --git a/lib/crewai/tests/skills/test_models_version.py b/lib/crewai/tests/skills/test_models_version.py index bea136fe6..262ca2a82 100644 --- a/lib/crewai/tests/skills/test_models_version.py +++ b/lib/crewai/tests/skills/test_models_version.py @@ -3,6 +3,7 @@ from __future__ import annotations import pytest +from pydantic import ValidationError from crewai.skills.models import SkillFrontmatter @@ -27,5 +28,5 @@ class TestSkillFrontmatterVersion: def test_frontmatter_is_frozen(self) -> None: fm = SkillFrontmatter(name="my-skill", description="A skill.", version="1.0.0") - with pytest.raises(Exception): + with pytest.raises(ValidationError): fm.version = "2.0.0" # type: ignore[misc]