diff --git a/api/skills_scan.py b/api/skills_scan.py new file mode 100644 index 000000000..c1ba6871a --- /dev/null +++ b/api/skills_scan.py @@ -0,0 +1,124 @@ +from __future__ import annotations + +import shutil +import time +import uuid +from pathlib import Path +from typing import Any + +from helpers import files, skills +from helpers.api import ApiHandler, Request, Response +from helpers.skills_import import extract_skills_zip +from werkzeug.datastructures import FileStorage +from werkzeug.utils import secure_filename + + +class SkillsScan(ApiHandler): + """ + Prepare skill scan targets for the Settings > Skills scanner. + """ + + async def process(self, input: dict[str, Any], request: Request) -> dict[str, Any] | Response: + if "skills_file" in request.files: + return self._prepare_uploaded_archive(request.files["skills_file"]) + + action = str(input.get("action") or "targets").strip().lower() + if action == "targets": + return self._list_installed_targets() + + return {"success": False, "error": "Invalid action"} + + def _list_installed_targets(self) -> dict[str, Any]: + targets: list[dict[str, Any]] = [] + seen: set[str] = set() + total_skills = 0 + + for raw_root in skills.get_skill_roots(): + root = Path(raw_root) + if not root.is_dir(): + continue + + skill_files = skills.discover_skill_md_files(root) + if not skill_files: + continue + + key = str(root.resolve()) + if key in seen: + continue + seen.add(key) + + skill_count = len(skill_files) + total_skills += skill_count + targets.append( + { + "path": str(root), + "display_path": files.normalize_a0_path(str(root)), + "skill_count": skill_count, + } + ) + + targets.sort(key=lambda item: item["path"]) + return { + "success": True, + "target_type": "installed", + "target_label": "Installed Agent Zero skills", + "targets": targets, + "paths": [item["path"] for item in targets], + "skill_count": total_skills, + } + + def _prepare_uploaded_archive(self, skills_file: FileStorage) -> dict[str, Any]: + if not skills_file.filename: + return {"success": False, "error": "No file selected"} + + base = secure_filename(skills_file.filename) # type: ignore[arg-type] + if not base.lower().endswith(".zip"): + return {"success": False, "error": "Skill scan uploads must be .zip files"} + + tmp_dir = Path(files.get_abs_path("tmp", "uploads")) + tmp_dir.mkdir(parents=True, exist_ok=True) + unique = uuid.uuid4().hex[:8] + stamp = time.strftime("%Y%m%d_%H%M%S") + tmp_path = tmp_dir / f"skills_scan_{stamp}_{unique}_{base}" + skills_file.save(str(tmp_path)) + + cleanup_root: Path | None = None + try: + scan_root, cleanup_root = extract_skills_zip( + tmp_path, + tmp_subdir="skill_scans", + prefix=f"scan_{unique}", + ) + skill_files = skills.discover_skill_md_files(scan_root) + skill_entries = [ + { + "path": str(skill_md.parent), + "relative_path": str(skill_md.parent.relative_to(scan_root)), + } + for skill_md in skill_files + ] + warnings = [] + if not skill_entries: + warnings.append("No SKILL.md files were found in the uploaded archive.") + + return { + "success": True, + "target_type": "uploaded_archive", + "target_label": base, + "paths": [str(scan_root)], + "scan_path": str(scan_root), + "display_path": files.normalize_a0_path(str(scan_root)), + "cleanup_paths": [str(cleanup_root)], + "skill_count": len(skill_entries), + "skills": skill_entries, + "warnings": warnings, + } + except Exception as exc: + if cleanup_root: + shutil.rmtree(cleanup_root, ignore_errors=True) + return {"success": False, "error": f"Failed to prepare skill scan: {exc}"} + finally: + try: + tmp_path.unlink(missing_ok=True) # type: ignore[arg-type] + except Exception: + pass diff --git a/api/skills_scan.py.dox.md b/api/skills_scan.py.dox.md new file mode 100644 index 000000000..b277ae73e --- /dev/null +++ b/api/skills_scan.py.dox.md @@ -0,0 +1,46 @@ +# skills_scan.py DOX + +## Purpose + +- Own the `skills_scan.py` API endpoint. +- Provide scan target discovery and uploaded skills archive preparation for the Settings > Skills scanner. +- Keep this file-level DOX profile synchronized with `skills_scan.py` because this directory is intentionally flat. + +## Ownership + +- `skills_scan.py` owns the runtime implementation. +- `skills_scan.py.dox.md` owns durable notes about responsibilities, contracts, side effects, and verification for that implementation. +- Classes: +- `SkillsScan` (`ApiHandler`) + - `async process(self, input: dict[str, Any], request: Request) -> dict[str, Any] | Response` + +## Runtime Contracts + +- HTTP handlers must derive from `helpers.api.ApiHandler`; WebSocket handlers must derive from `helpers.ws.WsHandler`. +- The JSON request action `targets` returns existing installed skill roots that contain at least one `SKILL.md`. +- Multipart requests with `skills_file` accept only `.zip` uploads, extract them into `tmp/skill_scans`, discover contained `SKILL.md` folders, and return `paths` plus `cleanup_paths` for the scanner prompt. +- Uploaded archives are not imported, installed, or executed by this endpoint. +- Temporary uploaded zip files under `tmp/uploads` are deleted after extraction or failure. +- Update this file whenever request payloads, authentication or CSRF requirements, response shapes, route side effects, or WebSocket event contracts change. +- Observed side-effect areas: filesystem reads, filesystem writes, filesystem deletion. +- Imported dependency areas include: `__future__`, `helpers`, `helpers.api`, `helpers.skills_import`, `pathlib`, `shutil`, `time`, `typing`, `uuid`, `werkzeug.datastructures`, `werkzeug.utils`. + +## Key Concepts + +- Installed target discovery uses `helpers.skills.get_skill_roots()` and filters to roots where `discover_skill_md_files()` finds skills. +- Uploaded zip preparation uses `extract_skills_zip()` so zip entries remain bounded to the temp extraction root. +- Response paths are local absolute paths for the scanner agent, while `display_path` provides normalized `/a0/...` style display when possible. + +## Work Guidance + +- Preserve authentication, CSRF, loopback, and API-key checks unless the endpoint contract explicitly changes. +- Do not execute uploaded files or scan targets in this endpoint. +- Keep temp extraction paths explicit so the LLM-driven scan prompt can clean them up. + +## Verification + +- Run endpoint-specific or API tests for changed behavior; smoke-test uploaded zip and installed-skill scan modal flows when practical. + +## Child DOX Index + +No child DOX files. diff --git a/helpers/skills_import.py b/helpers/skills_import.py index c73ffa55d..a043dcf3f 100644 --- a/helpers/skills_import.py +++ b/helpers/skills_import.py @@ -2,6 +2,7 @@ from __future__ import annotations import os import shutil +import stat import tempfile import time import zipfile @@ -82,25 +83,60 @@ def _candidate_skill_roots(source_dir: Path) -> List[Path]: return unique or [source_dir] +def _safe_extract_zip(zip_path: Path, target: Path) -> None: + with zipfile.ZipFile(zip_path, "r") as archive: + for member in archive.infolist(): + name = member.filename + if not name or name.startswith(("/", "\\")) or "\\" in name: + raise ValueError(f"Unsafe zip entry path: {name!r}") + + mode = member.external_attr >> 16 + if stat.S_ISLNK(mode): + raise ValueError(f"Refusing to extract symlink from zip: {name!r}") + + destination = target / name + if not _is_within(destination, target): + raise ValueError(f"Unsafe zip entry path: {name!r}") + + archive.extractall(target) + + +def extract_skills_zip( + zip_path: Path, + *, + tmp_subdir: str = "skill_imports", + prefix: str = "import", +) -> tuple[Path, Path]: + """ + Extract a zip into a temp folder inside Agent Zero's tmp directory. + Returns (source_root, cleanup_root). + """ + base_tmp = Path(files.get_abs_path("tmp", tmp_subdir)) + base_tmp.mkdir(parents=True, exist_ok=True) + stamp = time.strftime("%Y%m%d_%H%M%S") + target = base_tmp / f"{prefix}_{zip_path.stem}_{stamp}" + target.mkdir(parents=True, exist_ok=True) + + try: + _safe_extract_zip(zip_path, target) + except Exception: + shutil.rmtree(target, ignore_errors=True) + raise + + # If zip contains a single top-level folder, treat that as the root + children = [p for p in target.iterdir()] + if len(children) == 1 and children[0].is_dir(): + return children[0], target + return target, target + + def _unzip_to_temp_dir(zip_path: Path) -> Path: """ Extract a zip into a temp folder under tmp/skill_imports (inside Agent Zero base dir). Returns the extraction root folder. """ - base_tmp = Path(files.get_abs_path("tmp", "skill_imports")) - base_tmp.mkdir(parents=True, exist_ok=True) - stamp = time.strftime("%Y%m%d_%H%M%S") - target = base_tmp / f"import_{zip_path.stem}_{stamp}" - target.mkdir(parents=True, exist_ok=True) - - with zipfile.ZipFile(zip_path, "r") as z: - z.extractall(target) - - # If zip contains a single top-level folder, treat that as the root - children = [p for p in target.iterdir()] - if len(children) == 1 and children[0].is_dir(): - return children[0] - return target + source_root, _cleanup_root = extract_skills_zip(zip_path) + return source_root def build_import_plan( @@ -261,4 +297,3 @@ def import_skills( destination_root=dest_root, namespace=ns, ) - diff --git a/helpers/skills_import.py.dox.md b/helpers/skills_import.py.dox.md index 9da611849..e6bc0493e 100644 --- a/helpers/skills_import.py.dox.md +++ b/helpers/skills_import.py.dox.md @@ -17,6 +17,8 @@ - `_is_within(child: Path, parent: Path) -> bool` - `_derive_namespace(source: Path) -> str` - `_candidate_skill_roots(source_dir: Path) -> List[Path]`: Heuristics to find likely skill roots inside a repo/pack: +- `_safe_extract_zip(zip_path: Path, target: Path) -> None` +- `extract_skills_zip(zip_path: Path, tmp_subdir: str=..., prefix: str=...) -> tuple[Path, Path]`: Extract a zip into a temp folder inside Agent Zero's tmp directory and return the scan/import root plus cleanup root. - `_unzip_to_temp_dir(zip_path: Path) -> Path`: Extract a zip into a temp folder under tmp/skill_imports (inside Agent Zero base dir). - `build_import_plan(source: Path, dest_root: Path, namespace: Optional[str]=...) -> Tuple[List[ImportPlanItem], Path]`: Build a copy plan for importing skills from a source folder. - `_resolve_conflict(dest: Path, policy: ConflictPolicy) -> Tuple[Path, bool]`: Returns (final_dest_path, should_copy). @@ -32,11 +34,12 @@ - Helper modules own reusable framework APIs and must preserve public callers unless all callers, tests, and docs are updated together. - Update this file whenever public functions, classes, persistence behavior, path/security assumptions, side effects, or cross-module contracts change. - Observed side-effect areas: filesystem reads, filesystem writes, filesystem deletion, plugin state. -- Imported dependency areas include: `__future__`, `dataclasses`, `helpers`, `helpers.skills`, `os`, `pathlib`, `shutil`, `tempfile`, `time`, `typing`, `zipfile`. +- Imported dependency areas include: `__future__`, `dataclasses`, `helpers`, `helpers.skills`, `os`, `pathlib`, `shutil`, `stat`, `tempfile`, `time`, `typing`, `zipfile`. ## Key Concepts -- Important called helpers/classes observed in the source: `dataclass`, `strip`, `plugins.is_dir`, `Path`, `base_tmp.mkdir`, `time.strftime`, `target.mkdir`, `_candidate_skill_roots`, `Path.expanduser`, `resolve_skills_destination_root`, `dest_root.mkdir`, `build_import_plan`, `ImportResult`, `child.resolve.relative_to`, `direct.is_dir`, `discover_skill_md_files`, `plugins.iterdir`, `files.get_abs_path`, `zipfile.ZipFile`, `z.extractall`. +- Zip extraction rejects absolute paths, backslash paths, path traversal, and symlink entries before extraction. +- Important called helpers/classes observed in the source: `dataclass`, `strip`, `plugins.is_dir`, `Path`, `base_tmp.mkdir`, `time.strftime`, `target.mkdir`, `_candidate_skill_roots`, `Path.expanduser`, `resolve_skills_destination_root`, `dest_root.mkdir`, `build_import_plan`, `ImportResult`, `child.resolve.relative_to`, `direct.is_dir`, `discover_skill_md_files`, `plugins.iterdir`, `files.get_abs_path`, `zipfile.ZipFile`, `archive.extractall`, `shutil.rmtree`. - Keep request/response, tool, or helper semantics documented here at the same time as source changes. ## Work Guidance diff --git a/tests/test_skills_scan.py b/tests/test_skills_scan.py new file mode 100644 index 000000000..e0420aee3 --- /dev/null +++ b/tests/test_skills_scan.py @@ -0,0 +1,135 @@ +import importlib.util +import json +import sys +import types +import zipfile +from pathlib import Path + +import pytest + + +PROJECT_ROOT = Path(__file__).resolve().parents[1] +if str(PROJECT_ROOT) not in sys.path: + sys.path.insert(0, str(PROJECT_ROOT)) + + +def _load_skills_import_module(tmp_path): + module_name = "test_skills_import_module" + stub_names = ("helpers", "helpers.files", "helpers.skills") + missing = object() + originals = {name: sys.modules.get(name, missing) for name in stub_names} + + helpers_pkg = types.ModuleType("helpers") + helpers_pkg.__path__ = [] + + files = types.ModuleType("helpers.files") + files.get_abs_path = lambda *parts: str(tmp_path.joinpath(*parts)) + + skills = types.ModuleType("helpers.skills") + skills.discover_skill_md_files = lambda root: sorted(Path(root).rglob("SKILL.md")) + + helpers_pkg.files = files + helpers_pkg.skills = skills + sys.modules["helpers"] = helpers_pkg + sys.modules["helpers.files"] = files + sys.modules["helpers.skills"] = skills + + try: + spec = importlib.util.spec_from_file_location( + module_name, + PROJECT_ROOT / "helpers" / "skills_import.py", + ) + module = importlib.util.module_from_spec(spec) + assert spec and spec.loader + sys.modules[module_name] = module + spec.loader.exec_module(module) + return module + finally: + for name, original in originals.items(): + if original is missing: + sys.modules.pop(name, None) + else: + sys.modules[name] = original + + +def test_extract_skills_zip_rejects_path_traversal(monkeypatch, tmp_path): + skills_import = _load_skills_import_module(tmp_path) + + archive_path = tmp_path / "bad.zip" + with zipfile.ZipFile(archive_path, "w") as archive: + archive.writestr("../escape.txt", "nope") + + with pytest.raises(ValueError, match="Unsafe zip entry path"): + skills_import.extract_skills_zip(archive_path) + + assert not (tmp_path / "escape.txt").exists() + + +def test_extract_skills_zip_returns_single_top_level_root(monkeypatch, tmp_path): + skills_import = _load_skills_import_module(tmp_path) + + archive_path = tmp_path / "skills.zip" + with zipfile.ZipFile(archive_path, "w") as archive: + archive.writestr("pack/example/SKILL.md", "---\nname: Example\n---\nBody\n") + + source_root, cleanup_root = skills_import.extract_skills_zip(archive_path) + + assert source_root.name == "pack" + assert cleanup_root.is_dir() + assert (source_root / "example" / "SKILL.md").is_file() + + +def test_settings_skills_scan_section_and_prompt_assets_are_present(): + settings_store = ( + PROJECT_ROOT + / "webui" + / "components" + / "settings" + / "settings-store.js" + ).read_text(encoding="utf-8") + skills_settings = ( + PROJECT_ROOT + / "webui" + / "components" + / "settings" + / "skills" + / "skills-settings.html" + ).read_text(encoding="utf-8") + import_template = ( + PROJECT_ROOT + / "webui" + / "components" + / "settings" + / "skills" + / "import.html" + ).read_text(encoding="utf-8") + scan_prompt = ( + PROJECT_ROOT + / "webui" + / "components" + / "settings" + / "skills" + / "skill-scan-prompt.md" + ).read_text(encoding="utf-8") + scan_checks = json.loads( + ( + PROJECT_ROOT + / "webui" + / "components" + / "settings" + / "skills" + / "skill-scan-checks.json" + ).read_text(encoding="utf-8") + ) + scan_checks_text = json.dumps(scan_checks) + + assert "section-skills-scan" in settings_store + assert "settings/skills/scan.html" in skills_settings + assert "settings/skills/import.html" in skills_settings + assert "scanSelectedFile()" in import_template + assert "snyk-agent-scan@latest --json --no-bootstrap" in scan_prompt + assert "E004" in scan_checks_text + assert "W007" in scan_checks_text + assert "W008" in scan_checks_text + assert "W011" in scan_checks_text + assert "W012" in scan_checks_text diff --git a/webui/components/settings/AGENTS.md b/webui/components/settings/AGENTS.md index 7ea29a6d0..2f5529227 100644 --- a/webui/components/settings/AGENTS.md +++ b/webui/components/settings/AGENTS.md @@ -9,6 +9,7 @@ - `settings.html` and `settings-store.js` own the settings shell and state. - Subdirectories own settings areas such as agent, external, developer, MCP, backup, plugins, secrets, skills, tunnel, and A2A. - `mcp/client/` owns the global/project MCP server manager, server search, raw JSON editor surface, examples modal, server tool detail modal, MCP scanner modal, scan checks, and scan prompt assets. +- `skills/` owns skill listing, importing, standalone skill scanning, uploaded archive scan preparation UI, scanner modal, scan checks, and scan prompt assets. ## Local Contracts @@ -23,6 +24,7 @@ - Prefer subsection-local stores for complex settings areas. - Coordinate plugin settings UI changes with `webui/components/plugins/` and `plugins/AGENTS.md`. - Keep MCP scanner checks and prompt assets close to the MCP client modal so scanner behavior remains reviewable with the UI that invokes it. +- Keep Skills scanner checks and prompt assets close to the Skills settings section so scanner behavior remains reviewable with import and standalone scan entry points. - Keep MCP manager search and toggle affordances consistent between global and project scope because both are rendered by the same client modal. ## Verification diff --git a/webui/components/settings/settings-store.js b/webui/components/settings/settings-store.js index 59f983475..24f09ac25 100644 --- a/webui/components/settings/settings-store.js +++ b/webui/components/settings/settings-store.js @@ -34,6 +34,7 @@ const TAB_ITEMS = Object.freeze([ icon: "school", sections: [ { id: "section-skills-list", label: "List Skills", icon: "view_list" }, + { id: "section-skills-scan", label: "Scan Skills", icon: "radar" }, { id: "section-skills-import", label: "Import Skills", icon: "upload_file" }, ], }, diff --git a/webui/components/settings/skills/import.html b/webui/components/settings/skills/import.html index 9502d88cc..abc104382 100644 --- a/webui/components/settings/skills/import.html +++ b/webui/components/settings/skills/import.html @@ -67,6 +67,8 @@
+
+
+ +
+