From 7ac9910e1d380b920fcde51812039ba6673a4558 Mon Sep 17 00:00:00 2001 From: Alessandro <155005371+3clyp50@users.noreply.github.com> Date: Thu, 18 Jun 2026 12:52:41 +0200 Subject: [PATCH] Add skill scanner settings flow Add a Settings > Skills scanner section and modal backed by Snyk Agent Scan-oriented checks and prompts. Wire uploaded skill zips from Import Skills into the scanner before import, and add a preparation API that extracts archives into temporary scan roots without installing or executing them. Harden shared skill zip extraction against traversal, backslash paths, and symlink entries, and cover the new flow with focused scanner tests. --- api/skills_scan.py | 124 ++++++ api/skills_scan.py.dox.md | 46 ++ helpers/skills_import.py | 65 ++- helpers/skills_import.py.dox.md | 7 +- tests/test_skills_scan.py | 135 ++++++ webui/components/settings/AGENTS.md | 2 + webui/components/settings/settings-store.js | 1 + webui/components/settings/skills/import.html | 2 + webui/components/settings/skills/scan.html | 108 +++++ .../settings/skills/skill-scan-checks.json | 63 +++ .../settings/skills/skill-scan-prompt.md | 141 ++++++ .../settings/skills/skill-scan.html | 242 +++++++++++ .../settings/skills/skills-import-store.js | 13 +- .../settings/skills/skills-scan-store.js | 401 ++++++++++++++++++ .../settings/skills/skills-settings.html | 10 + 15 files changed, 1342 insertions(+), 18 deletions(-) create mode 100644 api/skills_scan.py create mode 100644 api/skills_scan.py.dox.md create mode 100644 tests/test_skills_scan.py create mode 100644 webui/components/settings/skills/scan.html create mode 100644 webui/components/settings/skills/skill-scan-checks.json create mode 100644 webui/components/settings/skills/skill-scan-prompt.md create mode 100644 webui/components/settings/skills/skill-scan.html create mode 100644 webui/components/settings/skills/skills-scan-store.js 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 @@
+
+
+ +
+