andre/crewAI
1
0
Fork 0
mirror of https://github.com/crewAIInc/crewAI.git synced 2026-07-03 14:09:24 +00:00
Code Issues Actions 4 Packages Projects Releases Wiki Activity

fix: address all CI failures and CodeRabbit review comments

Browse Source 
Lint:
- Remove unused imports (click, pytest, json)
- Replace try-except-pass with logging (S110)
- Fix unprotected zipfile.extractall (S202)

Security:
- Path traversal: startswith → is_relative_to for tar extraction
- Add path traversal protection to ZIP extraction via _safe_extract_zip
- Both cache.py and CLI main.py hardened

Type checker:
- Fix import path: crewai.events.event_bus (not crewai_event_bus)
- Remove unused type: ignore comments
- Fix type mismatches in set_skills() variable types

Code quality:
- Fix f-string interpolation in SkillNotCachedError
- Use ValidationError instead of Exception in test
This commit is contained in:
alex-clawd
2026-05-19 12:04:35 -07:00
parent 9965d67bb5
commit 3e0372dca0
7 changed files with 37 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
lib/cli/src/crewai_cli/skills/main.py
View File

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

1
lib/cli/tests/skills/test_main.py
View File

@@ -3,7 +3,6 @@
from __future__ import annotations
import io
import json
import os
import tempfile
import zipfile

4
lib/crewai/src/crewai/agent/core.py
View File

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

7
lib/crewai/src/crewai/skills/cache.py
View File

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

19
lib/crewai/src/crewai/skills/registry.py
View File

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

2
lib/crewai/tests/skills/test_cache.py
View File

@@ -8,8 +8,6 @@ import json
import tarfile
from pathlib import Path
import pytest
from crewai.skills.cache import SkillCacheManager

3
lib/crewai/tests/skills/test_models_version.py
View File

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