From e170c247925d239e052ae843b13e5702d4c5dabc Mon Sep 17 00:00:00 2001 From: jana-selva Date: Wed, 30 Sep 2026 16:43:47 +0530 Subject: [PATCH 1/3] Refactor skill installation helpers --- doc/changes/unreleased.md | 2 -- exasol/toolbox/util/skills.py | 37 +++++++++++++++++++++++++---------- test/unit/util/skill_test.py | 33 +++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 12 deletions(-) diff --git a/doc/changes/unreleased.md b/doc/changes/unreleased.md index 7e971f989..98534ea8a 100644 --- a/doc/changes/unreleased.md +++ b/doc/changes/unreleased.md @@ -3,7 +3,5 @@ ## Features * #940: Added shared validation for packaged agent skills and the `skills:check` Nox session. -* #938: Added the `skills:install` Nox session for installing the packaged PTB agent skill. * #942: Added api-contract-audit skill for identifying mismatches between type annotations, docstrings, and runtime behavior -* #963: Extended packaged skill checks and installation to support multiple skills. diff --git a/exasol/toolbox/util/skills.py b/exasol/toolbox/util/skills.py index 6eae1ff0d..7a12414e3 100644 --- a/exasol/toolbox/util/skills.py +++ b/exasol/toolbox/util/skills.py @@ -71,19 +71,16 @@ def _has_symlink_in_parents(path: Path) -> bool: return any(candidate.is_symlink() for candidate in (path, *path.parents)) -def install_skill( - skill_name: str = PTB_SKILL_NAME, - target_directory: Path | None = None, -) -> Path: - """Install a packaged skill into a project-local agent skill directory.""" +def _validate_skill_name(skill_name: str) -> None: + """Reject skill names that could escape the skill installation directory.""" if Path(skill_name).name != skill_name: raise ValueError(f"invalid skill name: {skill_name}") - source_files = get_skill_files(skill_name) - if not source_files: - raise ValueError(f"packaged skill does not exist: {skill_name}") - target_directory = target_directory or Path.cwd() / ".agents" / "skills" +def _prepare_installation_directory( + target_directory: Path, skill_name: str +) -> Path: + """Validate and recreate the destination directory for one skill.""" target_skill = target_directory / skill_name if _has_symlink_in_parents(target_directory): raise ValueError( @@ -97,10 +94,30 @@ def install_skill( if target_skill.exists(): shutil.rmtree(target_skill) target_skill.mkdir(parents=True, exist_ok=True) + return target_skill + + +def _copy_skill_files(source_files: Mapping[str, Traversable], target: Path) -> None: + """Copy packaged skill files below an already validated target directory.""" for relative_path, source in source_files.items(): - destination = target_skill / relative_path + destination = target / relative_path destination.parent.mkdir(parents=True, exist_ok=True) destination.write_bytes(source.read_bytes()) + + +def install_skill( + skill_name: str = PTB_SKILL_NAME, + target_directory: Path | None = None, +) -> Path: + """Install a packaged skill into a project-local agent skill directory.""" + _validate_skill_name(skill_name) + source_files = get_skill_files(skill_name) + if not source_files: + raise ValueError(f"packaged skill does not exist: {skill_name}") + + target_directory = target_directory or Path.cwd() / ".agents" / "skills" + target_skill = _prepare_installation_directory(target_directory, skill_name) + _copy_skill_files(source_files, target_skill) return target_skill diff --git a/test/unit/util/skill_test.py b/test/unit/util/skill_test.py index d96f254ba..c6c0b761e 100644 --- a/test/unit/util/skill_test.py +++ b/test/unit/util/skill_test.py @@ -1,6 +1,11 @@ import pytest from exasol.toolbox.util import skills +from exasol.toolbox.util.skills import ( + _copy_skill_files, + _prepare_installation_directory, + _validate_skill_name, +) def test_validate_skill_accepts_packaged_ptb_skill(): @@ -115,6 +120,34 @@ def test_install_skill_rejects_path_traversal(tmp_path): skills.install_skill("../outside", tmp_path) +def test_validate_skill_name_rejects_path_traversal(): + with pytest.raises(ValueError, match="invalid skill name"): + _validate_skill_name("nested/example") + + +def test_prepare_installation_directory_replaces_existing_directory(tmp_path): + target_directory = tmp_path / ".agents" / "skills" + target_skill = target_directory / "example" + target_skill.mkdir(parents=True) + (target_skill / "stale.md").write_text("stale", encoding="utf-8") + + prepared = _prepare_installation_directory(target_directory, "example") + + assert prepared == target_skill + assert not (target_skill / "stale.md").exists() + + +def test_copy_skill_files_copies_nested_files(tmp_path): + source = tmp_path / "source.md" + source.write_text("content", encoding="utf-8") + target = tmp_path / "target" + target.mkdir() + + _copy_skill_files({"references/source.md": source}, target) + + assert (target / "references/source.md").read_text(encoding="utf-8") == "content" + + def test_install_skill_rejects_missing_skill(tmp_path, monkeypatch): monkeypatch.setattr(skills, "get_skill_files", lambda _: {}) From 1d7f7124273f9581e33c2b9d0f06c6f55461fe7b Mon Sep 17 00:00:00 2001 From: jana-selva Date: Wed, 30 Sep 2026 16:49:15 +0530 Subject: [PATCH 2/3] format fix --- exasol/toolbox/util/skills.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/exasol/toolbox/util/skills.py b/exasol/toolbox/util/skills.py index 7a12414e3..febfc83ba 100644 --- a/exasol/toolbox/util/skills.py +++ b/exasol/toolbox/util/skills.py @@ -77,9 +77,7 @@ def _validate_skill_name(skill_name: str) -> None: raise ValueError(f"invalid skill name: {skill_name}") -def _prepare_installation_directory( - target_directory: Path, skill_name: str -) -> Path: +def _prepare_installation_directory(target_directory: Path, skill_name: str) -> Path: """Validate and recreate the destination directory for one skill.""" target_skill = target_directory / skill_name if _has_symlink_in_parents(target_directory): From ded951ffcec0f5eb9bef99404e1f1eac39982362 Mon Sep 17 00:00:00 2001 From: jana-selva Date: Wed, 30 Sep 2026 16:50:38 +0530 Subject: [PATCH 3/3] Update skill installation changelog --- doc/changes/unreleased.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/changes/unreleased.md b/doc/changes/unreleased.md index 98534ea8a..e350a4dd2 100644 --- a/doc/changes/unreleased.md +++ b/doc/changes/unreleased.md @@ -3,5 +3,8 @@ ## Features * #940: Added shared validation for packaged agent skills and the `skills:check` Nox session. +* #938: Added the `skills:install` Nox session for installing the packaged PTB agent skill. * #942: Added api-contract-audit skill for identifying mismatches between type annotations, docstrings, and runtime behavior +* #963: Extended packaged skill checks and installation to support multiple skills. +* #967: Refactored skill installation into reusable filesystem helpers.