feat: Skills Repository — registry, cache, CLI, and SDK integration#5867
Open
alex-clawd wants to merge 7 commits into
Open
feat: Skills Repository — registry, cache, CLI, and SDK integration#5867alex-clawd wants to merge 7 commits into
alex-clawd wants to merge 7 commits into
Conversation
Adds a Skills Repository feature allowing users to publish, install,
and use skills from the CrewAI registry with @org/skill-name refs.
## What's New
### SDK (lib/crewai/)
- SkillFrontmatter: added optional 'version' field (backward compatible)
- SkillCacheManager: manages ~/.crewai/skills/{org}/{name}/ with
.crewai_meta.json tracking, path-traversal-safe tar extraction
- SkillRegistry: parse @org/skill-name refs, local-first resolution
(./skills/ > cache > download), interactive prompt on first use,
CI-mode guard (CREWAI_NONINTERACTIVE/CI env vars)
- Agent.skills and Crew.skills widened to accept str refs (@org/name)
- set_skills() resolves registry refs with org-prefixed dedup keys
- New events: SkillDownloadStartedEvent, SkillDownloadCompletedEvent
### CLI (lib/cli/)
- crewai skill create <name> — context-aware (project vs standalone)
- crewai skill install @org/name — downloads to ./skills/ or cache
- crewai skill publish — ZIP + upload to org registry
- crewai skill list — show installed skills
### PlusAPI (lib/crewai-core/)
- Added SKILLS_RESOURCE, get_skill(), publish_skill(), list_skills()
### Scaffolding
- crew and flow templates now include skills/ directory
### Tests
- 91 SDK skill tests + 15 CLI skill tests, all passing
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughImplements a skill registry: parse/resolve ChangesCrewAI Agent Skills Registry
Sequence DiagramsequenceDiagram
participant User as User
participant SkillCLI as crewai skill (Click)
participant SkillCommand as SkillCommand
participant PlusAPI as PlusAPI
participant Cache as SkillCacheManager
participant Agent as Agent
User->>SkillCLI: run create/install/publish/list
SkillCLI->>SkillCommand: dispatch command
rect rgba(100, 150, 200, 0.5)
Note over SkillCommand: create -> scaffold directories, write SKILL.md
SkillCommand->>SkillCommand: create SKILL.md and assets
end
rect rgba(150, 100, 200, 0.5)
Note over SkillCommand: install -> fetch & cache
SkillCommand->>PlusAPI: get_skill(org, name)
PlusAPI-->>SkillCommand: base64 archive response
SkillCommand->>Cache: store(org, name, version, archive_bytes)
Cache-->>SkillCommand: cached_path
end
rect rgba(200, 150, 100, 0.5)
Note over SkillCommand: publish -> build ZIP and upload
SkillCommand->>SkillCommand: parse SKILL.md frontmatter
SkillCommand->>SkillCommand: build in-memory ZIP
SkillCommand->>PlusAPI: publish_skill(org, name, version, encoded_file)
PlusAPI-->>SkillCommand: publish response (status URL)
end
rect rgba(100, 200, 150, 0.5)
Note over SkillCommand: list -> aggregate installed skills
SkillCommand->>Cache: list_cached()
Cache-->>SkillCommand: [SkillMetadata...]
SkillCommand->>User: display Rich table
end
rect rgba(120, 120, 180, 0.5)
Note over Agent: runtime resolution of registry refs
Agent->>SkillCommand: resolve_registry_ref(`@org/name`)
SkillCommand->>Cache: get_cached_path(org, name)
alt cache miss & interactive
SkillCommand->>PlusAPI: get_skill(...)
PlusAPI-->>SkillCommand: archive
SkillCommand->>Cache: store(...)
end
SkillCommand-->>Agent: activated Skill
end
🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
lib/crewai/tests/skills/test_cache.py (1)
16-37: 💤 Low valueHelper function contains redundant archive creation.
The
_make_tar_gzfunction creates the archive twice (lines 18-27 are unused, then lines 30-37 create the actual output). The first block can be removed.Proposed simplification
def _make_tar_gz(files: dict[str, str]) -> bytes: """Build an in-memory .tar.gz containing the given filename → content mapping.""" - buf = io.BytesIO() - with gzip.GzipFile(fileobj=buf, mode="wb") as gz: - gz_buf = io.BytesIO() - with tarfile.open(fileobj=gz_buf, mode="w") as tf: - for name, content in files.items(): - data = content.encode() - info = tarfile.TarInfo(name=name) - info.size = len(data) - tf.addfile(info, io.BytesIO(data)) - gz.write(gz_buf.getvalue()) - buf.seek(0) - # Re-create properly: gzip wrapping a tar stream out = io.BytesIO() with tarfile.open(fileobj=out, mode="w:gz") as tf: for name, content in files.items(): data = content.encode() info = tarfile.TarInfo(name=name) info.size = len(data) tf.addfile(info, io.BytesIO(data)) return out.getvalue()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/tests/skills/test_cache.py` around lines 16 - 37, The helper _make_tar_gz currently builds the tar.gz archive twice: the first in-memory gzip/tar creation (using buf, gz, gz_buf and tf) is redundant and unused; remove that entire first block (everything involving buf, gzip.GzipFile, gz_buf and the inner tarfile loop) and keep the final tarfile.open(fileobj=out, mode="w:gz") section that writes the archive once and returns out.getvalue(), ensuring the function still accepts files: dict[str, str], encodes content to bytes, sets TarInfo.size, and adds files via tf.addfile as in the remaining code.lib/crewai-core/src/crewai_core/plus_api.py (1)
242-259: ⚡ Quick winConsider defining a TypedDict for the publish payload.
For consistency with the existing
PublishToolPayloadpattern and better type safety, consider defining aPublishSkillPayloadTypedDict:class PublishSkillPayload(TypedDict): org: str name: str version: str public: bool description: str | None file: strThen use it in the method signature:
def publish_skill(...) -> httpx.Response: payload: PublishSkillPayload = { "org": org, ... }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai-core/src/crewai_core/plus_api.py` around lines 242 - 259, Add a PublishSkillPayload TypedDict mirroring the existing PublishToolPayload pattern (fields: org, name, version, public, description, file) and annotate the local payload in publish_skill with that type; update the publish_skill function to declare payload: PublishSkillPayload = {...} so the payload construction uses the new TypedDict for stronger typing and consistency with other publish methods.lib/crewai/src/crewai/skills/models.py (1)
81-84: ⚡ Quick winConsider adding semantic version validation.
The description mentions "Semantic version of the skill, e.g. '1.0.0'", but there's no pattern validation. The
namefield has pattern validation viaSKILL_NAME_PATTERN. Consider whetherversionshould validate against a semver pattern for consistency:SEMVER_PATTERN = r"^\d+\.\d+\.\d+(?:-[a-zA-Z0-9.-]+)?(?:\+[a-zA-Z0-9.-]+)?$" version: str | None = Field( default=None, pattern=SEMVER_PATTERN, description="Semantic version of the skill, e.g. '1.0.0'. Optional for local skills.", )If flexible versioning is intentional, this can be skipped.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/skills/models.py` around lines 81 - 84, The version Field in models.py lacks semver validation despite its description; add a SEMVER_PATTERN constant (e.g. matching major.minor.patch with optional pre-release/build) and apply it to the existing version: str | None = Field(...) by adding pattern=SEMVER_PATTERN so the version value is validated; update imports if needed and ensure the symbol names SEMVER_PATTERN and the version Field are used exactly as in the diff.lib/cli/tests/skills/test_main.py (1)
83-120: ⚡ Quick winAdd a traversal regression test for install archive extraction.
Please add a test archive containing entries like
../pwned.txt(and absolute paths) and assertinstall()rejects it. This protects the extraction boundary from future regressions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/cli/tests/skills/test_main.py` around lines 83 - 120, Extend TestSkillInstall to add a traversal-regression test: modify or add a test (e.g., alongside test_install_in_project) that uses _zip_skill (or a new helper similar to _zip_skill) to produce a ZIP containing traversal entries like "../pwned.txt" and an absolute path (e.g., "/tmp/pwned.txt"), return it as a data:application/zip;base64,... mock response as in test_install_in_project, then call skill_command.install("`@acme/my-skill`") and assert the installer rejects the archive (e.g., raises SystemExit) and that no files outside the intended skills/my-skill directory are created; reference TestSkillInstall._zip_skill and skill_command.install when locating where to add the new test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/cli/src/crewai_cli/skills/main.py`:
- Around line 282-301: The archive extraction in _unpack_archive (and its TAR
helper _safe_extractall and the ZIP branch using ZipFile.extractall) is
vulnerable to path-traversal; replace the unvalidated extractall calls with
per-member path checks that resolve each member's destination path and ensure it
is inside dest using Path.resolve() and Path.relative_to(dest.resolve()) (raise
an error or skip on failure). For TAR: iterate tf.getmembers(), compute the
target path for each member (join dest with member.name), ensure
target.relative_to(dest.resolve()) succeeds, then use tf.extractfile or
tf.extract for safe extraction (preserving file types securely). For ZIP:
iterate zf.infolist(), compute each member's target path, validate with
Path.relative_to(dest.resolve()) and write files only after validation. Remove
any string-prefix startswith checks and use Path.relative_to() for ancestry
validation. Ensure symlinks or special files are handled safely (skip or
sanitize) and surface an error if any member fails validation.
In `@lib/crewai/src/crewai/skills/cache.py`:
- Around line 113-120: The path-traversal check in _safe_extractall is unsafe
because using str(...).startswith(dest_resolved) can be bypassed; update the
check to ensure each resolved member_path is actually inside dest_resolved by
using a proper path containment test (e.g., Path.is_relative_to(dest_resolved)
if available or using member_path.relative_to(dest_resolved) in a try/except to
detect escapes) before calling tf.extractall(dest), and raise the same
ValueError when the member is outside; keep the same function name
_safe_extractall and the tf.extractall call but replace the string-prefix test
with the robust containment test.
In `@lib/crewai/src/crewai/skills/registry.py`:
- Around line 16-24: The exception message in SkillNotCachedError.__init__ uses
a literal "{ref}" instead of interpolating the ref value; update the constructor
so the install hint includes the actual ref by making the second string an
f-string (or otherwise formatting it) so the combined message contains the real
ref for the user guidance in SkillNotCachedError.
- Around line 98-112: The bare "except: pass" blocks around the local and cached
skill loads (wrapping load_skill_metadata and activate_skill) hide real errors;
replace them with a narrowed exception handler or capture the exception as "e"
and log it before falling through. Specifically, in the try/except around
load_skill_metadata(local_path)/activate_skill(...) and the try/except around
load_skill_metadata(cached_path)/activate_skill(...), change "except Exception:
pass" to either "except (FileNotFoundError, PermissionError, ValueError,
ImportError) as e:" or "except Exception as e:" and call a logger (e.g.
logging.exception or an existing module logger) to record the error and context
(org/name/local_path or cached_path and SKILL.md) so failures are visible while
still allowing the code to proceed to the download branch.
In `@lib/crewai/tests/skills/test_models_version.py`:
- Around line 28-31: The test test_frontmatter_is_frozen should assert the
specific Pydantic error type instead of catching a broad Exception: change the
pytest.raises(Exception) to pytest.raises(ValidationError) and import
ValidationError from pydantic, ensuring the assertion uses the SkillFrontmatter
instance to trigger immutability (fm.version = "2.0.0") and that the failure is
specifically matched to ValidationError raised by the frozen model.
---
Nitpick comments:
In `@lib/cli/tests/skills/test_main.py`:
- Around line 83-120: Extend TestSkillInstall to add a traversal-regression
test: modify or add a test (e.g., alongside test_install_in_project) that uses
_zip_skill (or a new helper similar to _zip_skill) to produce a ZIP containing
traversal entries like "../pwned.txt" and an absolute path (e.g.,
"/tmp/pwned.txt"), return it as a data:application/zip;base64,... mock response
as in test_install_in_project, then call skill_command.install("`@acme/my-skill`")
and assert the installer rejects the archive (e.g., raises SystemExit) and that
no files outside the intended skills/my-skill directory are created; reference
TestSkillInstall._zip_skill and skill_command.install when locating where to add
the new test.
In `@lib/crewai-core/src/crewai_core/plus_api.py`:
- Around line 242-259: Add a PublishSkillPayload TypedDict mirroring the
existing PublishToolPayload pattern (fields: org, name, version, public,
description, file) and annotate the local payload in publish_skill with that
type; update the publish_skill function to declare payload: PublishSkillPayload
= {...} so the payload construction uses the new TypedDict for stronger typing
and consistency with other publish methods.
In `@lib/crewai/src/crewai/skills/models.py`:
- Around line 81-84: The version Field in models.py lacks semver validation
despite its description; add a SEMVER_PATTERN constant (e.g. matching
major.minor.patch with optional pre-release/build) and apply it to the existing
version: str | None = Field(...) by adding pattern=SEMVER_PATTERN so the version
value is validated; update imports if needed and ensure the symbol names
SEMVER_PATTERN and the version Field are used exactly as in the diff.
In `@lib/crewai/tests/skills/test_cache.py`:
- Around line 16-37: The helper _make_tar_gz currently builds the tar.gz archive
twice: the first in-memory gzip/tar creation (using buf, gz, gz_buf and tf) is
redundant and unused; remove that entire first block (everything involving buf,
gzip.GzipFile, gz_buf and the inner tarfile loop) and keep the final
tarfile.open(fileobj=out, mode="w:gz") section that writes the archive once and
returns out.getvalue(), ensuring the function still accepts files: dict[str,
str], encodes content to bytes, sets TarInfo.size, and adds files via tf.addfile
as in the remaining code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 524644bf-d2c5-4b17-bb66-5abd76eb2070
📒 Files selected for processing (19)
lib/cli/src/crewai_cli/cli.pylib/cli/src/crewai_cli/skills/__init__.pylib/cli/src/crewai_cli/skills/main.pylib/cli/src/crewai_cli/templates/crew/skills/.gitkeeplib/cli/src/crewai_cli/templates/flow/skills/.gitkeeplib/cli/tests/skills/__init__.pylib/cli/tests/skills/test_main.pylib/crewai-core/src/crewai_core/plus_api.pylib/crewai/src/crewai/agent/core.pylib/crewai/src/crewai/agents/agent_builder/base_agent.pylib/crewai/src/crewai/crew.pylib/crewai/src/crewai/events/types/skill_events.pylib/crewai/src/crewai/skills/__init__.pylib/crewai/src/crewai/skills/cache.pylib/crewai/src/crewai/skills/models.pylib/crewai/src/crewai/skills/registry.pylib/crewai/tests/skills/test_cache.pylib/crewai/tests/skills/test_models_version.pylib/crewai/tests/skills/test_registry.py
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
Contributor
Author
|
All 10 CodeRabbit comments addressed in commit 3e0372d:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/crewai/tests/skills/test_cache.py (1)
38-117: ⚡ Quick winAdd traversal-regression tests for malicious archive entries.
Current coverage is mostly happy-path. Please add explicit cases for
../and absolute-path entries (and ZIP variants ifstore()supports ZIP) to assert extraction is rejected and nothing is written outsidecache_root. This guards the recent path-traversal hardening from regressions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/tests/skills/test_cache.py` around lines 38 - 117, Tests currently lack cases ensuring SkillCacheManager.store is safe against path-traversal in archives; add unit tests that create malicious archives (tar.gz entries with "../" paths and absolute paths, and ZIP equivalents if SkillCacheManager.store accepts ZIP) and call SkillCacheManager.store(...) to assert the store call either rejects the archive (raises an exception) or sanitizes it by not writing any files outside the provided cache_root and not creating unexpected files; use get_cached_path(...) and inspect the tmp_path parent and siblings to assert nothing was written outside cache_root, and also verify that a successful store does not include files with traversal paths and that metadata is handled consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@lib/crewai/tests/skills/test_cache.py`:
- Around line 38-117: Tests currently lack cases ensuring
SkillCacheManager.store is safe against path-traversal in archives; add unit
tests that create malicious archives (tar.gz entries with "../" paths and
absolute paths, and ZIP equivalents if SkillCacheManager.store accepts ZIP) and
call SkillCacheManager.store(...) to assert the store call either rejects the
archive (raises an exception) or sanitizes it by not writing any files outside
the provided cache_root and not creating unexpected files; use
get_cached_path(...) and inspect the tmp_path parent and siblings to assert
nothing was written outside cache_root, and also verify that a successful store
does not include files with traversal paths and that metadata is handled
consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 75dd2f6e-e6b0-4908-a6a5-d1114e4ff08f
📒 Files selected for processing (7)
lib/cli/src/crewai_cli/skills/main.pylib/cli/tests/skills/test_main.pylib/crewai/src/crewai/agent/core.pylib/crewai/src/crewai/skills/cache.pylib/crewai/src/crewai/skills/registry.pylib/crewai/tests/skills/test_cache.pylib/crewai/tests/skills/test_models_version.py
🚧 Files skipped from review as they are similar to previous changes (6)
- lib/crewai/tests/skills/test_models_version.py
- lib/crewai/src/crewai/skills/cache.py
- lib/cli/src/crewai_cli/skills/main.py
- lib/crewai/src/crewai/skills/registry.py
- lib/crewai/src/crewai/agent/core.py
- lib/cli/tests/skills/test_main.py
There was a problem hiding this comment.
🧹 Nitpick comments (2)
lib/cli/src/crewai_cli/skills/main.py (2)
139-141: 💤 Low valueRemove unnecessary import —
plus_api_clientis already used at line 110.This import with
as _is dead code.self.plus_api_client.get_skill()is called at line 110 before theif in_project:branch, so ifcrewai_corewasn't available, the method would fail there. The comment "ensure crewai_core present" is misleading since that check happens implicitly earlier.🧹 Suggested cleanup
else: - from crewai_core.plus_api import ( - PlusAPI as _, # noqa: F401 (ensure crewai_core present) - ) - cache_dir = Path.home() / ".crewai" / "skills" / org / name🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/cli/src/crewai_cli/skills/main.py` around lines 139 - 141, Remove the dead-import that tries to "ensure crewai_core present" — delete the import block "from crewai_core.plus_api import PlusAPI as _ # noqa: F401" in skills/main.py because plus_api_client is already used earlier (self.plus_api_client.get_skill()) and the import is redundant and misleading; simply remove that import line and its comment so the code does not contain a no-op alias.
147-147: 💤 Low valueRemove redundant import —
jsonis already imported at line 7.Re-importing
json as _jsonis unnecessary sincejsonis already available at module scope. Use the existing import.🧹 Suggested fix
from datetime import datetime, timezone - import json as _json meta = { "org": org, "name": name, "version": version, "installed_at": datetime.now(tz=timezone.utc).isoformat(), } - (cache_dir / ".crewai_meta.json").write_text(_json.dumps(meta, indent=2)) + (cache_dir / ".crewai_meta.json").write_text(json.dumps(meta, indent=2))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/cli/src/crewai_cli/skills/main.py` at line 147, Remove the redundant import statement "import json as _json": delete that import and update any references to _json in this module to use the existing module-level "json" import instead so the single top-level import is used consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@lib/cli/src/crewai_cli/skills/main.py`:
- Around line 139-141: Remove the dead-import that tries to "ensure crewai_core
present" — delete the import block "from crewai_core.plus_api import PlusAPI as
_ # noqa: F401" in skills/main.py because plus_api_client is already used
earlier (self.plus_api_client.get_skill()) and the import is redundant and
misleading; simply remove that import line and its comment so the code does not
contain a no-op alias.
- Line 147: Remove the redundant import statement "import json as _json": delete
that import and update any references to _json in this module to use the
existing module-level "json" import instead so the single top-level import is
used consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9bc2b181-e205-4a5d-b205-5db5a1ad8826
📒 Files selected for processing (7)
lib/cli/src/crewai_cli/cli.pylib/cli/src/crewai_cli/git.pylib/cli/src/crewai_cli/skills/main.pylib/crewai-core/src/crewai_core/plus_api.pylib/crewai/src/crewai/agent/core.pylib/crewai/src/crewai/skills/cache.pylib/crewai/src/crewai/skills/registry.py
✅ Files skipped from review due to trivial changes (1)
- lib/cli/src/crewai_cli/git.py
🚧 Files skipped from review as they are similar to previous changes (5)
- lib/crewai/src/crewai/agent/core.py
- lib/crewai-core/src/crewai_core/plus_api.py
- lib/crewai/src/crewai/skills/cache.py
- lib/cli/src/crewai_cli/cli.py
- lib/crewai/src/crewai/skills/registry.py
- _parse_frontmatter() now delegates to crewai.skills.parser.parse_frontmatter when available, with a minimal fallback for CLI-only installs - install() global cache path now reuses SkillCacheManager.store() instead of duplicating metadata writing logic
…olCommand pattern)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/cli/src/crewai_cli/skills/main.py`:
- Around line 144-151: The fallback branch that unpacks to cache_dir (in main.py
around the except ImportError) must also write the same .crewai_meta.json
metadata that SkillCacheManager would produce so list_cached can discover the
skill; update the except block after self._unpack_archive(archive_bytes,
cache_dir) to create the metadata payload (including fields used by list_cached
such as ref, version, org, name, and any timestamps/ids the SDK path writes) and
atomically write it to cache_dir / ".crewai_meta.json" (mirroring the format
produced by SkillCacheManager); ensure the metadata keys and file name match
what list_cached expects so fallback-installed skills appear in crewi skill
list.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3f394393-5ccb-4f08-8bf0-653c3bc656b0
📒 Files selected for processing (1)
lib/cli/src/crewai_cli/skills/main.py
CodeRabbit caught that the ImportError fallback in install() didn't write cache metadata, making skills invisible to 'crewai skill list'.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
lib/cli/src/crewai_cli/skills/main.py (2)
300-304:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftThe pre-
filter="data"TAR fallback is still link-traversal unsafe.If
extractall(..., filter="data")hitsTypeError,_safe_extractall()only checksmember.nameand then hands the archive back totf.extractall(dest). A tar with an in-tree symlink or hardlink can still pivot later members outsidedeston runtimes that reach this fallback.Suggested fix direction
def _safe_extractall(tf: tarfile.TarFile, dest: Path) -> None: """Path-traversal-safe extraction for Python < 3.12.""" dest_resolved = dest.resolve() for member in tf.getmembers(): member_path = (dest / member.name).resolve() if not member_path.is_relative_to(dest_resolved): raise ValueError(f"Blocked path traversal attempt: {member.name!r}") - tf.extractall(dest) # noqa: S202 + if member.issym() or member.islnk() or not (member.isdir() or member.isfile()): + raise ValueError(f"Unsupported tar member: {member.name!r}") + if member.isdir(): + member_path.mkdir(parents=True, exist_ok=True) + continue + + member_path.parent.mkdir(parents=True, exist_ok=True) + src = tf.extractfile(member) + if src is None: + raise ValueError(f"Unreadable tar member: {member.name!r}") + member_path.write_bytes(src.read())Please verify whether this fallback is reachable in supported runtimes. If the repo still supports Python 3.10 or 3.11, this path remains live.
#!/bin/bash fd -HI '^(pyproject\.toml|setup\.cfg|setup\.py)$' . -t f | while read -r f; do echo "== $f ==" rg -n 'requires-python|python_requires|Programming Language :: Python ::' "$f" doneAlso applies to: 367-374
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/cli/src/crewai_cli/skills/main.py` around lines 300 - 304, The current fallback on except TypeError should not call tf.extractall because that reintroduces link-traversal risks; instead, replace the fallback so it uses a secure extraction path: call your existing _safe_extractall(tf, dest) (and ensure _safe_extractall does not delegate back to tf.extractall) and enhance _safe_extractall to explicitly reject or sanitize members that are symlinks or hard links (check TarInfo.issym()/islnk()), resolve member extraction targets with os.path.realpath(os.path.join(dest, member.name)) and verify the realpath starts with the intended dest realpath (reject if not), and handle link targets that point outside by skipping/raising; make the same change in the second occurrence (the other tarfile.extractall fallback block) so no fallback calls tf.extractall in any runtime.
145-150:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFallback installs still won't show up in
crewai skill list.
list_cached()only discovers global entries via.crewai_meta.json, but thisImportErrorbranch unpacks the archive and never writes that file. In CLI-only environments,installsucceeds and the nextlistcall silently misses the skill.Suggested fix
except ImportError: # Fallback if SDK not installed — write directly cache_dir = Path.home() / ".crewai" / "skills" / org / name cache_dir.mkdir(parents=True, exist_ok=True) self._unpack_archive(archive_bytes, cache_dir) + (cache_dir / ".crewai_meta.json").write_text( + json.dumps({"org": org, "name": name, "version": version}) + ) console.print(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/cli/src/crewai_cli/skills/main.py` around lines 145 - 150, The ImportError fallback unpacks the skill to cache_dir via _unpack_archive but doesn't create the .crewai_meta.json that list_cached() relies on, so add code in the same except ImportError branch (inside the install flow that calls _unpack_archive) to write the metadata file into cache_dir (e.g., ".crewai_meta.json") with the same fields/structure used by list_cached(), including org, name, version and any manifest/runtime fields list_cached expects; ensure it uses safe JSON serialization and sets file permissions as needed so CLI-only installs appear in list_cached().
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/cli/src/crewai_cli/skills/main.py`:
- Around line 94-106: Ensure the skill ref strictly matches the single-segment
`@org/name` contract and cannot escape paths: before using
without_at.partition("/") require without_at.count("/") == 1, then validate that
both org and name are non-empty, do not contain any slashes, do not contain the
path-traversal segment "..", and do not start with a dot (e.g.
org.startswith(".") or name.startswith(".")). Reject the ref (console.print +
SystemExit(1)) if any check fails; keep using the existing without_at, org and
name variables and then safely build paths like Path("skills") / name and the
cache path using those validated values.
---
Duplicate comments:
In `@lib/cli/src/crewai_cli/skills/main.py`:
- Around line 300-304: The current fallback on except TypeError should not call
tf.extractall because that reintroduces link-traversal risks; instead, replace
the fallback so it uses a secure extraction path: call your existing
_safe_extractall(tf, dest) (and ensure _safe_extractall does not delegate back
to tf.extractall) and enhance _safe_extractall to explicitly reject or sanitize
members that are symlinks or hard links (check TarInfo.issym()/islnk()), resolve
member extraction targets with os.path.realpath(os.path.join(dest, member.name))
and verify the realpath starts with the intended dest realpath (reject if not),
and handle link targets that point outside by skipping/raising; make the same
change in the second occurrence (the other tarfile.extractall fallback block) so
no fallback calls tf.extractall in any runtime.
- Around line 145-150: The ImportError fallback unpacks the skill to cache_dir
via _unpack_archive but doesn't create the .crewai_meta.json that list_cached()
relies on, so add code in the same except ImportError branch (inside the install
flow that calls _unpack_archive) to write the metadata file into cache_dir
(e.g., ".crewai_meta.json") with the same fields/structure used by
list_cached(), including org, name, version and any manifest/runtime fields
list_cached expects; ensure it uses safe JSON serialization and sets file
permissions as needed so CLI-only installs appear in list_cached().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1fbf6f74-4519-4abc-a094-90aae3871179
📒 Files selected for processing (1)
lib/cli/src/crewai_cli/skills/main.py
Reject refs with multiple slashes (@org/a/b), dot segments (@../skill), or leading dots in org/name. Applied to both CLI install() and SDK parse_registry_ref() so the contract is enforced consistently.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds a Skills Repository feature: users can publish, install, and use skills from the CrewAI registry using
@org/skill-namerefs.Why
Skills (
SKILL.mdinstruction bundles) currently only work as local filesystem paths. This adds a distribution layer so teams can:crewai skill publishcrewai skill install @org/skill-nameAgent(skills=['@acme/compliance-checker'])How It Works
Resolution order (local-first):
./skills/{name}/— project-local (editable, committed to repo)~/.crewai/skills/{org}/{name}/— global cacheCI/headless environments:
CI=1orCREWAI_NONINTERACTIVE=1→ no prompt, raisesSkillNotCachedErrorwith install instructions.Components
skills/cache.pySkillCacheManager—~/.crewai/skills/with.crewai_meta.json, path-traversal-safe extractionskills/registry.py@org/skillref parsing, resolution, interactive download promptskills/models.pyversionfield onSkillFrontmatter(optional, backward compat)skill_events.pySkillDownloadStartedEvent,SkillDownloadCompletedEventagent/core.py,base_agent.py,crew.pystr,set_skills()resolves registry refsplus_api.pyget_skill(),publish_skill(),list_skills()cli/skills/main.py,cli.pycrewai skill create/install/publish/listtemplates/crew/skills/,templates/flow/skills/skills/directoryUsage
Tests
Summary by CodeRabbit
New Features
crewai skillCLI for create, install, publish, and list workflows (project install, visibility, org, force options)@org/name) with automatic resolution and precedence for project-local skillsEvents
Tests