From f52818fe5ec00174691911fc6d89c4ad617027b1 Mon Sep 17 00:00:00 2001 From: Ryker_Feng <90562015+RykerFeng@users.noreply.github.com> Date: Fri, 11 Sep 2026 16:21:23 +0800 Subject: [PATCH] feat(skills): export custom skill packages with revision-bound preview (#5332) * feat(skills): export custom skill packages with revision preview * docs(gateway): keep export guidance within size budget * ci: retry checks after transient uv setup download failure * docs: focus skill export agent guidance on maintenance invariants * fix(skills): handle export disconnects and bound archive transfers * docs(gateway): remove redundant export guidance to fit merged budget * fix(skills): reset export idle deadline after transfer progress --- README.md | 6 + backend/app/gateway/routers/skills.py | 45 +- backend/app/gateway/skill_export.py | 188 +++++++ backend/docs/API.md | 11 + .../harness/deerflow/skills/AGENTS.md | 8 + .../harness/deerflow/skills/export.py | 482 +++++++++++++++++ .../harness/deerflow/skills/installer.py | 3 + .../harness/deerflow/skills/projection.py | 50 ++ .../skills/storage/local_skill_storage.py | 31 +- .../storage/user_scoped_skill_storage.py | 28 +- .../harness/deerflow/skills/validation.py | 6 + backend/scripts/benchmark/skill_export.py | 138 +++++ .../tests/blocking_io/test_skill_export.py | 38 ++ backend/tests/test_gateway_skill_export.py | 345 ++++++++++++ backend/tests/test_skill_export.py | 509 ++++++++++++++++++ frontend/AGENTS.md | 6 + .../settings/skill-export-dialog.tsx | 317 +++++++++++ .../settings/skill-settings-page.tsx | 34 +- frontend/src/core/i18n/locales/en-US.ts | 58 ++ frontend/src/core/i18n/locales/types.ts | 32 ++ frontend/src/core/i18n/locales/zh-CN.ts | 53 ++ frontend/src/core/skills/export.ts | 110 ++++ frontend/tests/e2e/skill-export.spec.ts | 144 +++++ .../settings/skill-export-dialog.dom.test.tsx | 129 +++++ .../tests/unit/core/skills/export.test.ts | 62 +++ 25 files changed, 2797 insertions(+), 36 deletions(-) create mode 100644 backend/app/gateway/skill_export.py create mode 100644 backend/packages/harness/deerflow/skills/export.py create mode 100644 backend/scripts/benchmark/skill_export.py create mode 100644 backend/tests/blocking_io/test_skill_export.py create mode 100644 backend/tests/test_gateway_skill_export.py create mode 100644 backend/tests/test_skill_export.py create mode 100644 frontend/src/components/workspace/settings/skill-export-dialog.tsx create mode 100644 frontend/src/core/skills/export.ts create mode 100644 frontend/tests/e2e/skill-export.spec.ts create mode 100644 frontend/tests/unit/components/workspace/settings/skill-export-dialog.dom.test.tsx create mode 100644 frontend/tests/unit/core/skills/export.test.ts diff --git a/README.md b/README.md index 415cbc51c..35b845f75 100644 --- a/README.md +++ b/README.md @@ -1173,6 +1173,12 @@ Web UI chat links percent-encode custom thread identifiers before placing them i └── lark-cli/lark-doc/SKILL.md ← managed, read-only ``` +#### Exporting Custom Skills + +Administrators can export their own custom skills from **Settings → Skills → Custom → Export**. Review the file list and declared environment requirements, then choose **Download .skill**. The archive contains the currently saved skill, including supporting files and empty directories; disabled skills can also be exported. If the skill changes after preview, refresh the file list before downloading. Import the archive on another DeerFlow instance with **Install .skill**; existing-name conflicts and normal installation security checks still apply. + +Account settings, conversations and history outside the skill folder are excluded. Files inside the folder are preserved unchanged, including any credentials an author placed there; filename notices are advisory. Configure dependencies and credentials on the destination. Linked folders/files, hard links, unsupported executable binaries, nested `SKILL.md` files and nonportable paths cannot be exported. Export supports hosts with descriptor-relative no-follow filesystem APIs (Linux/macOS); unsupported hosts fail explicitly. Limits: 4096 ZIP entries, 64 MiB per file, 100 MiB total content/archive and 1 MiB frontmatter. YAML aliases and excessively complex declarations are not supported. Ordinary script executable semantics are preserved on POSIX import, without restoring special permissions. See [the export API contract](backend/docs/API.md#export-a-custom-skill). + #### Claude Code Integration The `claude-to-deerflow` skill lets you interact with a running DeerFlow instance directly from [Claude Code](https://docs.anthropic.com/en/docs/claude-code). Send research tasks, check status, manage threads — all without leaving the terminal. diff --git a/backend/app/gateway/routers/skills.py b/backend/app/gateway/routers/skills.py index 02eedd965..02077e2f3 100644 --- a/backend/app/gateway/routers/skills.py +++ b/backend/app/gateway/routers/skills.py @@ -5,13 +5,14 @@ from collections.abc import AsyncGenerator from pathlib import Path from typing import BinaryIO, Literal -from fastapi import APIRouter, Depends, HTTPException, Request +from fastapi import APIRouter, Depends, HTTPException, Query, Request, Response from pydantic import BaseModel, Field from starlette.datastructures import FormData, Headers, UploadFile from starlette.formparsers import MultiPartException, MultiPartParser from app.gateway.deps import get_config, require_admin_user from app.gateway.path_utils import resolve_thread_virtual_path +from app.gateway.skill_export import ExportClientDisconnected, SkillExportManifestResponse, SkillExportResponse, export_http_error, run_export_work from deerflow.agents.lead_agent.prompt import clear_skills_system_prompt_cache, refresh_skills_system_prompt_cache_async, refresh_user_skills_system_prompt_cache_async from deerflow.config.app_config import AppConfig from deerflow.config.extensions_config import ( @@ -25,6 +26,7 @@ from deerflow.config.extensions_config import ( ) from deerflow.runtime.user_context import get_effective_user_id from deerflow.skills import Skill +from deerflow.skills.export import SkillExportError, build_skill_export, export_manifest from deerflow.skills.installer import SkillAlreadyExistsError, SkillSecurityScanError from deerflow.skills.security_scanner import scan_skill_content from deerflow.skills.security_static_scanner import ( @@ -388,6 +390,47 @@ async def list_custom_skills(config: AppConfig = Depends(get_config)) -> SkillsL raise HTTPException(status_code=500, detail=f"Failed to list custom skills: {str(e)}") +@router.get("/skills/custom/{skill_name}/export-manifest", response_model=SkillExportManifestResponse, response_model_exclude_unset=True, summary="Preview Custom Skill Export") +async def preview_custom_skill_export(skill_name: str, request: Request, response: Response, config: AppConfig = Depends(get_config)) -> SkillExportManifestResponse | Response: + await require_admin_user(request, detail=_ADMIN_REQUIRED_DETAIL) + try: + result, lease = await run_export_work(lambda cancel: export_manifest(_get_user_skill_storage(config), skill_name, cancel), request) + except ExportClientDisconnected: + return Response(status_code=204) + except SkillExportError as error: + raise export_http_error(error) from error + except HTTPException: + raise + except Exception: + raise HTTPException(500, detail={"code": "skill_export_failed", "message": "Could not prepare the skill export."}) from None + try: + response.headers["Cache-Control"] = "private, no-store" + return SkillExportManifestResponse.model_validate(result) + finally: + lease.release() + + +@router.get("/skills/custom/{skill_name}/export", summary="Download Custom Skill Archive") +async def download_custom_skill_export( + skill_name: str, + request: Request, + expected_revision: str = Query(..., pattern=r"^[a-f0-9]{64}$"), + config: AppConfig = Depends(get_config), +) -> Response: + await require_admin_user(request, detail=_ADMIN_REQUIRED_DETAIL) + try: + archive, lease = await run_export_work(lambda cancel: build_skill_export(_get_user_skill_storage(config), skill_name, expected_revision, cancel), request) + except ExportClientDisconnected: + return Response(status_code=204) + except SkillExportError as error: + raise export_http_error(error) from error + except HTTPException: + raise + except Exception: + raise HTTPException(500, detail={"code": "skill_export_failed", "message": "Could not prepare the skill export."}) from None + return SkillExportResponse(archive, skill_name, lease) + + @router.get("/skills/custom/{skill_name}", response_model=CustomSkillContentResponse, summary="Get Custom Skill Content") async def get_custom_skill(skill_name: str, request: Request, config: AppConfig = Depends(get_config)) -> CustomSkillContentResponse: await require_admin_user(request, detail=_ADMIN_REQUIRED_DETAIL) diff --git a/backend/app/gateway/skill_export.py b/backend/app/gateway/skill_export.py new file mode 100644 index 000000000..fdaffe028 --- /dev/null +++ b/backend/app/gateway/skill_export.py @@ -0,0 +1,188 @@ +"""Request-owned export workers and responses; slots follow temporary-file lifetime.""" + +from __future__ import annotations + +import asyncio +import threading +from collections.abc import Callable +from typing import Any, Literal + +from fastapi import HTTPException, Request +from pydantic import BaseModel +from starlette.requests import ClientDisconnect +from starlette.responses import StreamingResponse + +from deerflow.skills.export import SkillExportArchive, SkillExportError +from deerflow.utils.file_io import run_file_io + +# Slots are shared across all users in this Gateway process. +_slots = threading.BoundedSemaphore(2) +TRANSFER_IDLE_TIMEOUT_SECONDS = 120.0 + + +class ExportClientDisconnected(Exception): + """A peer disconnect, distinct from cancellation of the server task.""" + + +class SkillExportNotice(BaseModel): + code: str + message: str + path: str | None = None + + +class SkillExportFile(BaseModel): + path: str + type: Literal["file", "directory"] + size: int + executable: bool + + +class SkillExportSecret(BaseModel): + name: str + optional: bool + + +class SkillExportRequirements(BaseModel): + compatibility: str | None + allowed_tools: list[str] | None + required_secrets: list[SkillExportSecret] | None + + +class SkillExportManifestResponse(BaseModel): + skill_name: str + revision: str | None + can_export: bool + file_count: int + directory_count: int + total_bytes: int + files: list[SkillExportFile] + requirements: SkillExportRequirements + warnings: list[SkillExportNotice] + blockers: list[SkillExportNotice] + + +class ExportLease: + def __init__(self) -> None: + self._released = False + + @classmethod + def acquire(cls) -> ExportLease: + if not _slots.acquire(blocking=False): + raise HTTPException(429, detail={"code": "skill_export_busy", "message": "Both export slots in this Gateway process are in use across all users. Retry after an export finishes."}) + return cls() + + def release(self) -> None: + if not self._released: + self._released = True + _slots.release() + + +async def _drain(task: asyncio.Task) -> None: + # Cancelling an asyncio future never stops its file-I/O worker. Repeated + # cancellations must not release the slot or close a file still in use. + while not task.done(): + try: + await asyncio.shield(task) + except asyncio.CancelledError: + continue + except Exception: + break + + +async def _finish_io(func: Callable, *args): + task = asyncio.create_task(run_file_io(func, *args)) + try: + return await asyncio.shield(task) + except asyncio.CancelledError: + await _drain(task) + if not task.cancelled(): + task.exception() + raise + + +async def _disconnected(request: Request) -> None: + while (await request.receive())["type"] != "http.disconnect": + pass + + +async def run_export_work(work: Callable[[threading.Event], Any], request: Request | None = None) -> tuple[Any, ExportLease]: + """Transfer a successful result AND lease, or drain/clean them before raising.""" + lease = ExportLease.acquire() + cancel_event = threading.Event() + task = asyncio.create_task(run_file_io(work, cancel_event)) + disconnected = asyncio.create_task(_disconnected(request)) if request is not None else None + try: + if disconnected is not None: + done, _ = await asyncio.wait((task, disconnected), return_when=asyncio.FIRST_COMPLETED) + if disconnected in done: + raise ExportClientDisconnected + return await asyncio.shield(task), lease + except BaseException: + cancel_event.set() + await _drain(task) + try: + if not task.cancelled(): + try: + result = task.result() + except BaseException: + pass + else: + if isinstance(result, SkillExportArchive): + await _finish_io(result.close) + finally: + lease.release() + raise + finally: + if disconnected is not None: + disconnected.cancel() + await _drain(disconnected) + + +class SkillExportResponse(StreamingResponse): + """Own cleanup even if ASGI fails before it starts iterating the body.""" + + def __init__(self, archive: SkillExportArchive, name: str, lease: ExportLease) -> None: + self.archive, self.lease = archive, lease + super().__init__( + self._chunks(), + media_type="application/zip", + headers={ + "Content-Disposition": f'attachment; filename="{name}.skill"', + "Content-Length": str(archive.size), + "Cache-Control": "private, no-store", + "X-Content-Type-Options": "nosniff", + }, + ) + + async def _chunks(self): + while chunk := await _finish_io(self.archive.file.read, 1024 * 1024): + yield chunk + + async def __call__(self, scope, receive, send) -> None: + try: + try: + async with asyncio.timeout(TRANSFER_IDLE_TIMEOUT_SECONDS) as idle_timeout: + + async def send_with_progress(message): + await send(message) + # Reset only after transport acceptance, not merely after + # reading another chunk. Healthy slow clients can finish. + idle_timeout.reschedule(asyncio.get_running_loop().time() + TRANSFER_IDLE_TIMEOUT_SECONDS) + + await super().__call__(scope, receive, send_with_progress) + except TimeoutError: + # Headers may already be sent. Abort the incomplete transfer; + # never report success or append JSON to a partial ZIP. + raise ClientDisconnect from None + finally: + try: + await _finish_io(self.archive.close) + finally: + self.lease.release() + + +def export_http_error(error: SkillExportError) -> HTTPException: + detail = {"code": error.code, "message": error.message} + if error.path is not None: + detail["path"] = error.path + return HTTPException(error.status, detail=detail) diff --git a/backend/docs/API.md b/backend/docs/API.md index 3284e9fe6..73230a868 100644 --- a/backend/docs/API.md +++ b/backend/docs/API.md @@ -755,6 +755,17 @@ Content-Type: multipart/form-data } ``` +#### Export a Custom Skill + +Admin session authentication is required for both requests. PAT credentials cannot export. Only the current user's custom skill is eligible; public, legacy and integration fallback is never used. A disabled custom skill remains eligible. + +1. `GET /api/skills/custom/{skill_name}/export-manifest` returns `skill_name`, `revision` (SHA-256 or null), `can_export`, `file_count`, `directory_count`, `total_bytes`, `files` (`path`, `type`, `size`, `executable`), `requirements` (`compatibility`, `allowed_tools`, `required_secrets` names and optional flags), and structured `warnings`/`blockers`. Paths are relative; `.` is the package root, counted in directory/entry totals. Structural blockers return a non-downloadable manifest. Declarations are not credential values or dependency verification. +2. `GET /api/skills/custom/{skill_name}/export?expected_revision=<64 lowercase hex characters>` recaptures content and rejects stale previews with 409 before sending ZIP headers. Successful responses carry `application/zip`, attachment `.skill`, accurate `Content-Length`, `Cache-Control: private, no-store`, and `X-Content-Type-Options: nosniff`. + +Error `detail` contains a safe `code`, `message`, and optional relative `path`. Codes/statuses: `skill_not_found` 404, `skill_changed` 409, `skill_export_limit_exceeded` 413, `skill_export_unsupported` 422, `skill_export_busy` 429, `skill_export_timeout` 503, `skill_export_failed` 500; existing 401/403 auth behavior applies. Limits are 4096 entries including directories, 64 MiB/file, 100 MiB raw/ZIP, 1 MiB frontmatter, 1024 UTF-8 bytes per ZIP path and depth 32. Frontmatter preflight rejects YAML aliases and bounds structure to 32 nesting levels / 16384 parser events before constructing YAML objects. A 5-second lock wait and 60-second cooperative worker deadline bound work; blocking OS calls cannot be forcibly interrupted. Two export slots are shared across all users in each Gateway process; both previews and downloads use them, and 429 means that process-wide capacity is occupied. Slots remain held through worker drain and temporary-file cleanup. The streaming phase has a separate 120-second inactivity deadline, reset after each successful ASGI send. A continuously progressing transfer may exceed 120 seconds overall; a stalled send does not reset the deadline. Expiry aborts the incomplete download (no replacement JSON after ZIP headers); clients must retry. Client disconnect during preparation cancels and drains the worker, then exits the handler normally rather than leaking a synthetic task cancellation. No export cache, persistent job or sharing URL is created. + +Raw skill files, sidecars and empty directories are preserved. No hooks/scripts run during export and no secrets are redacted from package files. Import still uses normal security scanning and conflict checks. Export requires no-follow descriptor-relative host filesystem operations; unsupported platforms receive 422 rather than following links unsafely. + #### Reload Skills Invalidate the skill prompt caches for every user in the current Gateway diff --git a/backend/packages/harness/deerflow/skills/AGENTS.md b/backend/packages/harness/deerflow/skills/AGENTS.md index 266915633..72e52eeb3 100644 --- a/backend/packages/harness/deerflow/skills/AGENTS.md +++ b/backend/packages/harness/deerflow/skills/AGENTS.md @@ -30,3 +30,11 @@ Lets a caller pass per-request, short-lived end-user credentials (e.g. an ERP to - **Leak surfaces sealed** (verified by a real-gateway e2e run — secret reaches the sandbox but none of these): prompt (value never in a message), trace (`tracing/metadata.py` never copies `context`), checkpoint (secrets live on `runtime.context`, not graph state), audit (journal records names only), stdout (`tools.py::mask_secret_values` redacts injected values from bash output), and **run-record persistence + run API** (`services.py::start_run` stores `redact_config_secrets(body.config)` so `runs.kwargs_json` and `RunResponse.kwargs` never carry the secret). - **Historical retention**: API response hiding prevents legacy `metadata.auth_token` and `config.metadata.auth_token` from being returned now; it does not delete values already retained in databases, run events, logs, snapshots, exports, or backups. Deployments that ever used either legacy carrier must rotate the credential and clean every retained copy under their retention policy. Restarting or upgrading DeerFlow performs neither action. - **Scope / non-goals**: no persistence/vaulting — values are request-scoped and never stored server-side, so long-lived use means the caller re-supplies `context.secrets` on each request while the skill stays in `skill_context`; subagents do not inherit the skill injection set. MCP interceptors may independently consume the same supported request-scoped carrier. Tests: `tests/test_skill_request_scoped_secrets.py`, `tests/test_mcp_session_pool.py`. + +### Custom skill export + +- `export.py` captures only `storage.get_custom_skill_dir(name)`; never use public or legacy fallback. Export neither executes skills nor replaces installation scanning. +- Capture and recheck source bytes under `skill_projection_read_lock` in `projection.py`, using the same lock as storage mutations. Keep writer staging and cleanup inside that lock; read-only export must not rebuild projections. +- Build archives from captured bytes and require the preview's revision. Preserve file contents, empty directories, and normalized executable flags; import must not restore privileged permission bits. +- Keep traversal, YAML parsing, and archive construction bounded and cancellable. Reject unsupported filesystem operations rather than following links; report limits instead of truncating results. Preserve YAML preflight before object construction. +- Export includes raw saved files and is not a secret audit. Return relative paths and generic errors without leaking source content. See the [export API contract](../../../../docs/API.md#export-a-custom-skill) for response fields and limits. diff --git a/backend/packages/harness/deerflow/skills/export.py b/backend/packages/harness/deerflow/skills/export.py new file mode 100644 index 000000000..1633372bd --- /dev/null +++ b/backend/packages/harness/deerflow/skills/export.py @@ -0,0 +1,482 @@ +"""Bounded custom-skill export; snapshots never activate or execute a skill.""" + +from __future__ import annotations + +import codecs +import errno +import hashlib +import os +import re +import stat +import struct +import tempfile +import time +import unicodedata +import zipfile +from dataclasses import dataclass +from pathlib import Path +from typing import BinaryIO + +import yaml + +from deerflow.skills.frontmatter import _FRONTMATTER_RE, split_skill_markdown +from deerflow.skills.installer import is_executable_binary_prefix +from deerflow.skills.parser import parse_allowed_tools +from deerflow.skills.projection import skill_projection_read_lock +from deerflow.skills.validation import validate_skill_frontmatter_text + +MAX_ENTRIES = 4096 +MAX_FILE_BYTES = 64 * 1024 * 1024 +MAX_TOTAL_BYTES = 100 * 1024 * 1024 +MAX_ZIP_BYTES = 100 * 1024 * 1024 +MAX_PATH_BYTES = 1024 +MAX_DEPTH = 32 +MAX_FRONTMATTER_BYTES = 1024 * 1024 +MAX_YAML_EVENTS = 16384 +MAX_YAML_DEPTH = 32 +DEADLINE_SECONDS = 60.0 +LOCK_TIMEOUT_SECONDS = 5.0 +CHUNK_SIZE = 65536 +_REVISION = re.compile(r"^[a-f0-9]{64}$") +_NAME = re.compile(r"^[a-z0-9]+(?:-[a-z0-9]+)*$") +_RESERVED = re.compile(r"^(?:con|prn|aux|nul|com[1-9¹²³]|lpt[1-9¹²³])(?:\.|$)", re.IGNORECASE) +_SENSITIVE = {".env", ".npmrc", ".pypirc", ".netrc", ".git", ".svn", ".hg", "credentials.json", "id_rsa", "id_ed25519"} + + +class SkillExportError(Exception): + """Safe public diagnostics, with no host paths or source text.""" + + def __init__(self, status: int, code: str, message: str, path: str | None = None): + super().__init__(message) + self.status = status + self.code = code + self.message = message + self.path = path + + +@dataclass +class SkillExportArchive: + file: BinaryIO + size: int + + def close(self): + self.file.close() + + +@dataclass(frozen=True) +class _Entry: + path: str + type: str + size: int + executable: bool + digest: bytes = b"" + offset: int = 0 + identity: tuple = () + + +class _Budget: + def __init__(self, cancel_event): + self.deadline = time.monotonic() + DEADLINE_SECONDS + self.cancel_event = cancel_event + + def check(self): + if self.cancel_event is not None and self.cancel_event.is_set(): + raise SkillExportError(503, "skill_export_cancelled", "Skill export was cancelled.") + if time.monotonic() >= self.deadline: + raise SkillExportError(503, "skill_export_timeout", "Skill export timed out.") + + +def _limit(): + raise SkillExportError(413, "skill_export_limit_exceeded", "Skill exceeds an export resource limit.") + + +def _changed(): + raise SkillExportError(409, "skill_changed", "Skill changed; refresh the file manifest.") + + +def _issue(code, message, path=None): + result = {"code": code, "message": message} + if path is not None: + # Invalid filenames may contain control chars; never echo those diagnostics. + result["path"] = "".join(c if c.isprintable() else "\ufffd" for c in path)[:1024] + return result + + +def _identity(info): + return (info.st_dev, info.st_ino, info.st_mode, info.st_nlink, info.st_size, info.st_mtime_ns, info.st_ctime_ns) + + +def _link(info): + return stat.S_ISLNK(info.st_mode) or bool(getattr(info, "st_file_attributes", 0) & 0x400) + + +def _invalid_path(path): + return any(p in ("", ".", "..") or p.endswith((" ", ".")) or _RESERVED.match(p) or any(unicodedata.category(c) == "Cc" or c in '\\<>:"|?*' for c in p) for p in path.split("/")) + + +def _walk(root_fd, root_stat, snapshot, budget, skill_name): + entries = [_Entry("", "directory", 0, False, identity=_identity(root_stat))] + blockers = [] + warnings = [] + folded = set() + total = 0 + visited = 1 + + def visit(directory_fd, parent): + nonlocal total, visited + budget.check() + # scandir iterator prevents allocating an unbounded directory listing. + children = [] + with os.scandir(directory_fd) as iterator: + for child in iterator: + budget.check() + if visited + len(children) >= MAX_ENTRIES: + _limit() + children.append(child.name) + for name in sorted(children, key=lambda s: s.encode("utf-8", "surrogatepass")): + budget.check() + visited += 1 + if visited > MAX_ENTRIES: + _limit() + path = parent + "/" + name if parent else name + try: + encoded = path.encode("utf-8") + except UnicodeEncodeError: + blockers.append(_issue("skill_export_invalid_path", "Path is not valid Unicode.", path)) + continue + if len(skill_name.encode("utf-8")) + 1 + len(encoded) > MAX_PATH_BYTES or len(path.split("/")) > MAX_DEPTH: + _limit() + if len(entries) >= MAX_ENTRIES: + _limit() + key = unicodedata.normalize("NFC", path).casefold() + if _invalid_path(path) or key in folded: + blockers.append(_issue("skill_export_invalid_path", "Path is not portable or conflicts with another path.", path)) + folded.add(key) + lower = name.casefold() + if lower in _SENSITIVE or lower.startswith(".env."): + warnings.append(_issue("skill_export_sensitive_filename", "This filename may contain local credentials or repository metadata; review it before sharing.", path)) + info = os.stat(name, dir_fd=directory_fd, follow_symlinks=False) + identity = _identity(info) + if _link(info): + blockers.append(_issue("skill_export_link", "Linked files or directories cannot be exported.", path)) + entries.append(_Entry(path, "file", 0, False, identity=identity)) + continue + if stat.S_ISDIR(info.st_mode): + if len(skill_name.encode("utf-8")) + 2 + len(encoded) > MAX_PATH_BYTES: + _limit() + entries.append(_Entry(path, "directory", 0, False, identity=identity)) + fd = os.open(name, os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW, dir_fd=directory_fd) + try: + if _identity(os.fstat(fd)) != identity: + _changed() + visit(fd, path) + if _identity(os.fstat(fd)) != identity: + _changed() + finally: + os.close(fd) + continue + if not stat.S_ISREG(info.st_mode) or info.st_nlink != 1: + blockers.append(_issue("skill_export_unsupported_node", "Only regular files with one link are supported.", path)) + entries.append(_Entry(path, "file", 0, False, identity=identity)) + continue + if info.st_size > MAX_FILE_BYTES or total + info.st_size > MAX_TOTAL_BYTES: + _limit() + if name == "SKILL.md" and parent: + blockers.append(_issue("skill_export_nested_skill", "Nested SKILL.md files are not accepted by the installer.", path)) + fd = os.open(name, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK, dir_fd=directory_fd) + try: + if _identity(os.fstat(fd)) != identity: + _changed() + digest = hashlib.sha256() + length = 0 + decoder = codecs.getincrementaldecoder("utf-8")() if path == "SKILL.md" else None + offset = snapshot.tell() if snapshot is not None else 0 + while True: + budget.check() + chunk = os.read(fd, CHUNK_SIZE) + if not chunk: + break + if length == 0 and is_executable_binary_prefix(chunk): + blockers.append(_issue("skill_export_executable_binary", "Executable binary files are not accepted by the installer.", path)) + if decoder is not None: + try: + decoder.decode(chunk, final=False) + except UnicodeError: + blockers.append(_issue("skill_export_invalid_frontmatter", "SKILL.md must be valid UTF-8.", path)) + decoder = None + length += len(chunk) + total += len(chunk) + if length > MAX_FILE_BYTES or total > MAX_TOTAL_BYTES: + _limit() + digest.update(chunk) + if snapshot is not None: + snapshot.write(chunk) + if decoder is not None: + try: + decoder.decode(b"", final=True) + except UnicodeError: + blockers.append(_issue("skill_export_invalid_frontmatter", "SKILL.md must be valid UTF-8.", path)) + if length != info.st_size or _identity(os.fstat(fd)) != identity: + _changed() + entries.append(_Entry(path, "file", length, bool(info.st_mode & 0o111), digest.digest(), offset, identity)) + finally: + os.close(fd) + + visit(root_fd, "") + if _identity(os.fstat(root_fd)) != _identity(root_stat): + _changed() + return sorted(entries, key=lambda e: e.path.encode("utf-8", "surrogatepass")), blockers, warnings + + +def _revision(name, entries): + digest = hashlib.sha256() + + def field(value): + digest.update(struct.pack(">Q", len(value))) + digest.update(value) + + field(b"deerflow-skill-export-v1") + field(name.encode("utf-8")) + for entry in entries: + for value in (entry.path.encode("utf-8"), entry.type.encode("ascii"), str(entry.size).encode("ascii"), entry.digest, b"1" if entry.executable else b"0"): + field(value) + return digest.hexdigest() + + +class _UnsupportedFrontmatter(ValueError): + """Contains only constant public explanations, never parser exceptions.""" + + def __init__(self, message, code="skill_export_yaml_complexity"): + super().__init__(message) + self.code = code + + +def _guard_frontmatter(source, budget): + """Inspect events without constructing aliases or expanding YAML merge keys.""" + budget.check() + depth = 0 + events = yaml.parse(source, Loader=yaml.SafeLoader) + try: + for count, event in enumerate(events, start=1): + budget.check() + if count > MAX_YAML_EVENTS: + raise _UnsupportedFrontmatter("YAML frontmatter exceeds the supported structural complexity.") + if isinstance(event, yaml.events.AliasEvent): + raise _UnsupportedFrontmatter("YAML aliases are not supported for skill export.", "skill_export_yaml_alias") + if isinstance(event, (yaml.events.MappingStartEvent, yaml.events.SequenceStartEvent)): + depth += 1 + if depth > MAX_YAML_DEPTH: + raise _UnsupportedFrontmatter("YAML frontmatter exceeds the supported nesting depth.") + elif isinstance(event, (yaml.events.MappingEndEvent, yaml.events.SequenceEndEvent)): + depth -= 1 + finally: + events.close() + budget.check() + + +def _manifest(name, entries, blockers, warnings, snapshot, budget): + if _invalid_path(name): + blockers.append(_issue("skill_export_invalid_path", "Skill root name is not portable.", ".")) + requirements = {"compatibility": None, "allowed_tools": None, "required_secrets": None} + skill = next((entry for entry in entries if entry.path == "SKILL.md" and entry.type == "file" and entry.digest), None) + if skill is None: + blockers.append(_issue("skill_export_invalid_frontmatter", "A regular root SKILL.md is required.", "SKILL.md")) + else: + snapshot.seek(skill.offset) + try: + prefix = snapshot.read(min(skill.size, MAX_FRONTMATTER_BYTES + 1)) + content = codecs.getincrementaldecoder("utf-8")().decode(prefix, final=skill.size <= len(prefix)) + match = _FRONTMATTER_RE.match(content) + if len(prefix) > MAX_FRONTMATTER_BYTES and (match is None or len(match.group(1).encode("utf-8")) > MAX_FRONTMATTER_BYTES): + _limit() + # Validation only uses frontmatter; a large body stays in the raw snapshot. + if match is not None: + content = content[: match.end()] + _guard_frontmatter(match.group(1), budget) + valid, _, declared_name = validate_skill_frontmatter_text(content) + if not valid or declared_name != name: + blockers.append(_issue("skill_export_invalid_frontmatter", "SKILL.md frontmatter must be valid and its name must match the directory.", "SKILL.md")) + else: + parts, _ = split_skill_markdown(content) + metadata = parts.metadata + compatibility = metadata.get("compatibility") + requirements["compatibility"] = compatibility if isinstance(compatibility, str) else None + tools = metadata.get("allowed-tools") + requirements["allowed_tools"] = list(parse_allowed_tools(tools, Path("SKILL.md"))) if tools is not None else None + secrets = metadata.get("required-secrets") + if secrets is not None: + requirements["required_secrets"] = [] + seen = set() + for secret in secrets: + if isinstance(secret, str): + secret = {"name": secret, "optional": False} + if isinstance(secret, dict): + raw_name = secret.get("name") + secret_name = raw_name.strip() if isinstance(raw_name, str) else "" + if re.fullmatch(r"[A-Za-z_][A-Za-z0-9_]*", secret_name) and secret_name not in seen: + seen.add(secret_name) + requirements["required_secrets"].append({"name": secret_name, "optional": bool(secret.get("optional", False))}) + elif not secret_name or not re.fullmatch(r"[A-Za-z_][A-Za-z0-9_]*", secret_name): + warnings.append(_issue("skill_export_invalid_declaration", "A malformed required-secrets declaration was omitted; inspect SKILL.md.", "SKILL.md")) + else: + warnings.append(_issue("skill_export_invalid_declaration", "A malformed required-secrets declaration was omitted; inspect SKILL.md.", "SKILL.md")) + if any(key in metadata for key in ("required-secrets", "secrets-autonomous", "allowed-tools")): + warnings.append(_issue("skill_export_platform_declarations", "Declared tools and secrets require configuration in the target environment.", "SKILL.md")) + except _UnsupportedFrontmatter as error: + blockers.append(_issue(error.code, str(error), "SKILL.md")) + except (UnicodeError, ValueError, TypeError, RecursionError, yaml.YAMLError): + blockers.append(_issue("skill_export_invalid_frontmatter", "SKILL.md frontmatter is not supported.", "SKILL.md")) + return { + "skill_name": name, + "revision": None if blockers else _revision(name, entries), + "can_export": not blockers, + "file_count": sum(e.type == "file" for e in entries), + "directory_count": sum(e.type == "directory" for e in entries), + "total_bytes": sum(e.size for e in entries), + "files": [{"path": e.path or ".", "type": e.type, "size": e.size, "executable": e.executable} for e in entries], + "requirements": requirements, + "warnings": warnings, + "blockers": blockers, + } + + +class _LinkedDirectory(Exception): + pass + + +def _open_directory_chain(path, budget): + """Anchor every component, including custom-root ancestors, without following links.""" + path = path.absolute() + descriptor = os.open(path.anchor, os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW) + try: + for component in path.parts[1:]: + budget.check() + info = os.stat(component, dir_fd=descriptor, follow_symlinks=False) + if _link(info): + raise _LinkedDirectory() + child = os.open(component, os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW, dir_fd=descriptor) + os.close(descriptor) + descriptor = child + if _identity(os.fstat(descriptor)) != _identity(info): + _changed() + return descriptor + except BaseException: + os.close(descriptor) + raise + + +def _capture(storage, name, snapshot, budget): + if not isinstance(name, str) or len(name) > 64 or not _NAME.fullmatch(name): + raise SkillExportError(422, "skill_export_unsupported", "Invalid skill name.") + root = storage.get_custom_skill_dir(name) + budget.check() + try: + with skill_projection_read_lock(storage, timeout=LOCK_TIMEOUT_SECONDS, check=budget.check): + if not hasattr(os, "O_NOFOLLOW") or os.open not in os.supports_dir_fd or os.scandir not in os.supports_fd: + raise SkillExportError(422, "skill_export_unsupported", "This platform cannot safely capture skill files.") + parent_fd = _open_directory_chain(root.parent, budget) + try: + info = os.stat(name, dir_fd=parent_fd, follow_symlinks=False) + if _link(info): + return [], [_issue("skill_export_link", "Linked skill directories cannot be exported.")], [] + if not stat.S_ISDIR(info.st_mode): + return [], [_issue("skill_export_unsupported_node", "Skill root must be a directory.")], [] + root_fd = os.open(name, os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW, dir_fd=parent_fd) + try: + if _identity(os.fstat(root_fd)) != _identity(info): + _changed() + entries, blockers, warnings = _walk(root_fd, info, snapshot, budget, name) + checked, checked_blockers, _ = _walk(root_fd, info, None, budget, name) + if [(e.path, e.identity, e.digest) for e in entries] != [(e.path, e.identity, e.digest) for e in checked] or checked_blockers != blockers: + _changed() + if _identity(os.stat(name, dir_fd=parent_fd, follow_symlinks=False)) != _identity(info): + _changed() + if _identity(root.parent.lstat()) != _identity(os.fstat(parent_fd)): + _changed() + return entries, blockers, warnings + finally: + os.close(root_fd) + finally: + os.close(parent_fd) + except _LinkedDirectory: + return [], [_issue("skill_export_link", "Linked skill directories cannot be exported.")], [] + except FileNotFoundError: + if not root.exists(): + raise SkillExportError(404, "skill_not_found", "Custom skill not found.") from None + _changed() + except TimeoutError: + raise SkillExportError(503, "skill_export_timeout", "Skill export lock timed out.") from None + except OSError as error: + if error.errno in (errno.ELOOP, errno.ENOTDIR): + _changed() + raise SkillExportError(500, "skill_export_failed", "Unable to read skill files.") from None + + +class _LimitedWriter: + def __init__(self, file, budget): + self.file = file + self.budget = budget + + def write(self, data): + self.budget.check() + if self.file.tell() + len(data) > MAX_ZIP_BYTES: + _limit() + return self.file.write(data) + + def __getattr__(self, name): + return getattr(self.file, name) + + +def export_manifest(storage, skill_name, cancel_event=None): + budget = _Budget(cancel_event) + with tempfile.TemporaryFile("w+b") as snapshot: + entries, blockers, warnings = _capture(storage, skill_name, snapshot, budget) + result = _manifest(skill_name, entries, blockers, warnings, snapshot, budget) + budget.check() + return result + + +def build_skill_export(storage, skill_name, expected_revision, cancel_event=None): + if not isinstance(expected_revision, str) or not _REVISION.fullmatch(expected_revision): + raise SkillExportError(422, "skill_export_unsupported", "A valid expected revision is required.") + budget = _Budget(cancel_event) + with tempfile.TemporaryFile("w+b") as snapshot: + entries, blockers, warnings = _capture(storage, skill_name, snapshot, budget) + manifest = _manifest(skill_name, entries, blockers, warnings, snapshot, budget) + if blockers: + raise SkillExportError(422, "skill_export_unsupported", "Skill contains unsupported files or structure.", blockers[0].get("path")) + if manifest["revision"] != expected_revision: + _changed() + output = tempfile.TemporaryFile("w+b") + try: + with zipfile.ZipFile(_LimitedWriter(output, budget), "w", compression=zipfile.ZIP_DEFLATED) as archive: + for entry in entries: + budget.check() + name = skill_name + "/" + entry.path + directory = entry.type == "directory" + if directory and not name.endswith("/"): + name += "/" + info = zipfile.ZipInfo(name, date_time=(1980, 1, 1, 0, 0, 0)) + info.create_system = 3 + info.compress_type = zipfile.ZIP_DEFLATED + mode = (stat.S_IFDIR | 0o755) if directory else (stat.S_IFREG | (0o755 if entry.executable else 0o644)) + info.external_attr = (mode << 16) | (0x10 if directory else 0) + with archive.open(info, "w") as target: + snapshot.seek(entry.offset) + remaining = entry.size + while remaining: + budget.check() + chunk = snapshot.read(min(CHUNK_SIZE, remaining)) + if not chunk: + raise SkillExportError(500, "skill_export_failed", "Unable to build skill archive.") + target.write(chunk) + remaining -= len(chunk) + size = output.tell() + if size > MAX_ZIP_BYTES: + _limit() + budget.check() + output.seek(0) + return SkillExportArchive(output, size) + except BaseException: + output.close() + raise diff --git a/backend/packages/harness/deerflow/skills/installer.py b/backend/packages/harness/deerflow/skills/installer.py index a5020ee33..c4ac5b123 100644 --- a/backend/packages/harness/deerflow/skills/installer.py +++ b/backend/packages/harness/deerflow/skills/installer.py @@ -7,6 +7,7 @@ Both Gateway and Client delegate to these functions. import asyncio import concurrent.futures import logging +import os import posixpath import shutil import stat @@ -191,6 +192,8 @@ def safe_extract_skill_archive( if total_written > max_total_size: raise ValueError("Skill archive is too large or appears highly compressed.") dst.write(chunk) + if os.name == "posix": + member_path.chmod(0o755 if (info.external_attr >> 16) & 0o111 else 0o644) def _is_script_support_file(rel_path: Path) -> bool: diff --git a/backend/packages/harness/deerflow/skills/projection.py b/backend/packages/harness/deerflow/skills/projection.py index f33a6558d..dcccad474 100644 --- a/backend/packages/harness/deerflow/skills/projection.py +++ b/backend/packages/harness/deerflow/skills/projection.py @@ -813,3 +813,53 @@ def ensure_public_skill_projection(*, app_config=None) -> bool: logger.error("Failed to clear the public skill projection after a boot-time error", exc_info=True) return False return True + + +@contextmanager +def skill_projection_read_lock(storage: SkillStorage, *, timeout: float = 5.0, check=None) -> Iterator[None]: + """Bounded, non-mutating acquisition of the existing user projection lock.""" + import time + + root = (storage.get_skills_root_path() / "custom") if getattr(storage, "user_id", None) is None else get_skill_projection_paths(storage).custom.parent + lock_path = root.parent / f".{root.name}.projection.lock" + lock_path.parent.mkdir(parents=True, exist_ok=True) + process_lock = _lock_for(lock_path) + deadline = time.monotonic() + timeout + acquired = False + try: + while not acquired: + if check: + check() + acquired = process_lock.acquire(timeout=min(0.05, max(0, deadline - time.monotonic()))) + if not acquired and time.monotonic() >= deadline: + raise TimeoutError("Skill projection lock timeout") + with lock_path.open("a", encoding="utf-8") as lock_file: + locked = False + try: + while not locked: + if check: + check() + try: + if fcntl is not None: + fcntl.flock(lock_file, fcntl.LOCK_EX | fcntl.LOCK_NB) + else: # pragma: no cover - Windows + lock_file.seek(0) + msvcrt.locking(lock_file.fileno(), msvcrt.LK_NBLCK, 1) + locked = True + except OSError as error: + if error.errno not in (errno.EACCES, errno.EAGAIN, errno.EDEADLK): + raise + if time.monotonic() >= deadline: + raise TimeoutError("Skill projection lock timeout") from None + time.sleep(0.02) + yield + finally: + if locked: + if fcntl is not None: + fcntl.flock(lock_file, fcntl.LOCK_UN) + else: # pragma: no cover - Windows + lock_file.seek(0) + msvcrt.locking(lock_file.fileno(), msvcrt.LK_UNLCK, 1) + finally: + if acquired: + process_lock.release() diff --git a/backend/packages/harness/deerflow/skills/storage/local_skill_storage.py b/backend/packages/harness/deerflow/skills/storage/local_skill_storage.py index e0ffc0a81..f276c13e5 100644 --- a/backend/packages/harness/deerflow/skills/storage/local_skill_storage.py +++ b/backend/packages/harness/deerflow/skills/storage/local_skill_storage.py @@ -10,7 +10,6 @@ import os import shutil import tempfile from collections.abc import Iterable -from contextlib import nullcontext from datetime import UTC, datetime from pathlib import Path @@ -98,23 +97,19 @@ class LocalSkillStorage(SkillStorage): return (self.get_custom_skill_dir(name) / SKILL_MD_FILE).read_text(encoding="utf-8") def write_custom_skill(self, name: str, relative_path: str, content: str) -> None: - target = self.validate_relative_path(relative_path, self.get_custom_skill_dir(name)) - target.parent.mkdir(parents=True, exist_ok=True) - with tempfile.NamedTemporaryFile( - "w", - encoding="utf-8", - delete=False, - dir=str(target.parent), - ) as tmp_file: - tmp_file.write(content) - tmp_path = Path(tmp_file.name) - try: - with self._skill_projection_mutation(): + with self._skill_projection_mutation(): + target = self.validate_relative_path(relative_path, self.get_custom_skill_dir(name)) + target.parent.mkdir(parents=True, exist_ok=True) + tmp_path = None + try: + with tempfile.NamedTemporaryFile("w", encoding="utf-8", delete=False, dir=str(target.parent)) as tmp_file: + tmp_path = Path(tmp_file.name) + tmp_file.write(content) tmp_path.replace(target) make_skill_written_path_sandbox_readable(self.get_custom_skill_dir(name), target) - except Exception: - tmp_path.unlink(missing_ok=True) - raise + finally: + if tmp_path is not None: + tmp_path.unlink(missing_ok=True) def remove_custom_skill_file(self, name: str, relative_path: str) -> str: removal = ((SkillCategory.CUSTOM, Path(name)),) @@ -246,7 +241,9 @@ class LocalSkillStorage(SkillStorage): remove_names: tuple[str, ...] = (), ): if getattr(self, "user_id", None) is None: - return nullcontext() + from deerflow.skills.projection import _projection_lock + + return _projection_lock(self.get_skills_root_path() / "custom") from deerflow.skills.projection import skill_projection_mutation return skill_projection_mutation(self, "user", remove=remove, remove_names=remove_names) diff --git a/backend/packages/harness/deerflow/skills/storage/user_scoped_skill_storage.py b/backend/packages/harness/deerflow/skills/storage/user_scoped_skill_storage.py index 923ea0b9c..e7a652039 100644 --- a/backend/packages/harness/deerflow/skills/storage/user_scoped_skill_storage.py +++ b/backend/packages/harness/deerflow/skills/storage/user_scoped_skill_storage.py @@ -362,25 +362,19 @@ class UserScopedSkillStorage(LocalSkillStorage): # ------------------------------------------------------------------ def write_custom_skill(self, name: str, relative_path: str, content: str) -> None: - # Ensure user custom skills directory exists - self._user_custom_root.mkdir(parents=True, exist_ok=True) - target = self.validate_relative_path(relative_path, self.get_custom_skill_dir(name)) - target.parent.mkdir(parents=True, exist_ok=True) - with tempfile.NamedTemporaryFile( - "w", - encoding="utf-8", - delete=False, - dir=str(target.parent), - ) as tmp_file: - tmp_file.write(content) - tmp_path = Path(tmp_file.name) - try: - with self._skill_projection_mutation(): + with self._skill_projection_mutation(): + target = self.validate_relative_path(relative_path, self.get_custom_skill_dir(name)) + target.parent.mkdir(parents=True, exist_ok=True) + tmp_path = None + try: + with tempfile.NamedTemporaryFile("w", encoding="utf-8", delete=False, dir=str(target.parent)) as tmp_file: + tmp_path = Path(tmp_file.name) + tmp_file.write(content) tmp_path.replace(target) make_skill_written_path_sandbox_readable(self.get_custom_skill_dir(name), target) - except Exception: - tmp_path.unlink(missing_ok=True) - raise + finally: + if tmp_path is not None: + tmp_path.unlink(missing_ok=True) # ------------------------------------------------------------------ # Public helpers diff --git a/backend/packages/harness/deerflow/skills/validation.py b/backend/packages/harness/deerflow/skills/validation.py index 3da808a4a..43361ff02 100644 --- a/backend/packages/harness/deerflow/skills/validation.py +++ b/backend/packages/harness/deerflow/skills/validation.py @@ -25,6 +25,12 @@ def _validate_skill_frontmatter(skill_dir: Path) -> tuple[bool, str, str | None] return False, f"{SKILL_MD_FILE} not found", None content = skill_md.read_text(encoding="utf-8") + return validate_skill_frontmatter_text(content) + + +def validate_skill_frontmatter_text(content: str) -> tuple[bool, str, str | None]: + """Validate captured text using the same rules as installation.""" + skill_md = Path(SKILL_MD_FILE) parts, error = split_skill_markdown(content) if error: return False, error, None diff --git a/backend/scripts/benchmark/skill_export.py b/backend/scripts/benchmark/skill_export.py new file mode 100644 index 000000000..cd5d23fff --- /dev/null +++ b/backend/scripts/benchmark/skill_export.py @@ -0,0 +1,138 @@ +"""Reproducible bounded export measurements against production entry points. + +Run from backend with PYTHONPATH=packages/harness:packages/extension-api:.: + python scripts/benchmark/skill_export.py +Each workload uses a fresh process; RSS is that process's peak, including imports. +No scripts, network, models, or security scanners are invoked by this benchmark. +""" + +from __future__ import annotations + +import json +import os +import resource +import shutil +import subprocess +import sys +import tempfile +import threading +import time +from pathlib import Path + +from deerflow.skills import export +from deerflow.skills.storage.local_skill_storage import LocalSkillStorage + + +class Tracker: + def __init__(self): + self.files = [] + self.peak_count = 0 + self.peak_bytes = 0 + self.original = tempfile.TemporaryFile + + def sample(self): + active = [file for file in self.files if not file.closed] + self.peak_count = max(self.peak_count, len(active)) + self.peak_bytes = max(self.peak_bytes, sum(file.high_water for file in active)) + + def create(self, *args, **kwargs): + tracker = self + + class Tracked: + def __init__(self, raw): + self.raw = raw + self.high_water = 0 + + def write(self, data): + written = self.raw.write(data) + self.high_water = max(self.high_water, self.raw.tell()) + tracker.sample() + return written + + def __getattr__(self, name): + return getattr(self.raw, name) + + def __enter__(self): + return self + + def __exit__(self, *args): + self.raw.close() + + file = Tracked(self.original(*args, **kwargs)) + self.files.append(file) + self.sample() + return file + + +def run(case): + with tempfile.TemporaryDirectory(prefix="skill-export-benchmark-") as workspace: + storage = LocalSkillStorage(host_path=workspace) + name = "skill-creator" if case == "public" else "benchmark" + root = storage.get_custom_skill_dir(name) + if case == "public": + public = Path(__file__).resolve().parents[3] / "skills/public/skill-creator" + shutil.copytree(public, root, ignore=shutil.ignore_patterns("__pycache__")) + else: + root.mkdir(parents=True) + (root / "SKILL.md").write_text("---\nname: benchmark\ndescription: Export benchmark\n---\n", encoding="utf-8") + if case in ("large", "cancel"): + # Incompressible bytes exercise ZIP disk limits realistically. + with (root / "asset.bin").open("wb") as file: + for _ in range(64): + file.write(os.urandom(1024 * 1024)) + elif case == "entries": + for index in range(4094): + (root / f"asset-{index:04}").touch() + tracker = Tracker() + export.tempfile.TemporaryFile = tracker.create + start = time.perf_counter() + manifest_seconds = None + archive_bytes = None + outcome = "ok" + try: + if case == "cancel": + event = threading.Event() + timer = threading.Timer(0.02, event.set) + timer.start() + try: + export.export_manifest(storage, name, event) + except export.SkillExportError as error: + outcome = error.code + finally: + timer.cancel() + timer.join() + else: + manifest = export.export_manifest(storage, name) + manifest_seconds = time.perf_counter() - start + archive = export.build_skill_export(storage, name, manifest["revision"]) + try: + archive_bytes = archive.size + finally: + archive.close() + elapsed = time.perf_counter() - start + finally: + export.tempfile.TemporaryFile = tracker.original + rss = resource.getrusage(resource.RUSAGE_SELF).ru_maxrss + print( + json.dumps( + { + "case": case, + "outcome": outcome, + "wall_seconds": round(elapsed, 4), + "manifest_seconds": round(manifest_seconds, 4) if manifest_seconds is not None else None, + "peak_rss_bytes": rss if sys.platform == "darwin" else rss * 1024, + "peak_temp_files": tracker.peak_count, + "peak_temp_bytes": tracker.peak_bytes, + "remaining_temp_files": sum(not file.closed for file in tracker.files), + "archive_bytes": archive_bytes, + } + ) + ) + + +if __name__ == "__main__": + if len(sys.argv) > 1: + run(sys.argv[1]) + else: + for case in ("public", "large", "entries", "cancel"): + subprocess.run([sys.executable, __file__, case], check=True) diff --git a/backend/tests/blocking_io/test_skill_export.py b/backend/tests/blocking_io/test_skill_export.py new file mode 100644 index 000000000..24f255870 --- /dev/null +++ b/backend/tests/blocking_io/test_skill_export.py @@ -0,0 +1,38 @@ +"""The real exporter, compression, response reads and cleanup stay off-loop.""" + +import asyncio + +import pytest + +from app.gateway.skill_export import SkillExportResponse, run_export_work +from deerflow.skills.export import build_skill_export, export_manifest +from deerflow.skills.storage.local_skill_storage import LocalSkillStorage + + +@pytest.fixture +def storage(tmp_path): + storage = LocalSkillStorage(host_path=str(tmp_path)) + root = storage.get_custom_skill_dir("demo") + root.mkdir(parents=True) + (root / "SKILL.md").write_text("---\nname: demo\ndescription: Offline fixture\n---\nbody", encoding="utf-8") + return storage + + +@pytest.mark.asyncio +async def test_export_worker_and_response_are_off_loop(storage): + manifest, lease = await run_export_work(lambda cancel: export_manifest(storage, "demo", cancel)) + lease.release() + archive, lease = await run_export_work(lambda cancel: build_skill_export(storage, "demo", manifest["revision"], cancel)) + response = SkillExportResponse(archive, "demo", lease) + sent = [] + + async def send(event): + sent.append(event) + + async def receive(): + await asyncio.Event().wait() + + await response({"type": "http", "asgi": {"spec_version": "2.4"}}, receive, send) + assert sent[0]["status"] == 200 + assert b"".join(event.get("body", b"") for event in sent).startswith(b"PK") + assert archive.file.closed diff --git a/backend/tests/test_gateway_skill_export.py b/backend/tests/test_gateway_skill_export.py new file mode 100644 index 000000000..e3659fa4e --- /dev/null +++ b/backend/tests/test_gateway_skill_export.py @@ -0,0 +1,345 @@ +"""Real archive/router contracts; auth is stamped only for this isolated test app.""" + +import asyncio +import threading +from io import BytesIO +from types import SimpleNamespace +from uuid import uuid4 +from zipfile import ZipFile + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient +from starlette.requests import ClientDisconnect + +from app.gateway import skill_export as service +from app.gateway.auth.models import User +from app.gateway.deps import get_config +from app.gateway.routers import skills +from deerflow.skills.export import SkillExportArchive +from deerflow.skills.storage.user_scoped_skill_storage import UserScopedSkillStorage + + +@pytest.fixture +def app(tmp_path, monkeypatch): + from deerflow.config import paths + + monkeypatch.setattr(paths, "_paths", paths.Paths(base_dir=tmp_path / "home")) + stores = {u: UserScopedSkillStorage(u, host_path=str(tmp_path / "skills")) for u in ("alice", "bob")} + for u, store in stores.items(): + root = store.get_custom_skill_dir("demo") + root.mkdir(parents=True) + (root / "SKILL.md").write_text(f"---\nname: demo\ndescription: {u}\n---\n{u}", encoding="utf-8") + app = FastAPI() + + @app.middleware("http") + async def identity(request, call_next): + if request.headers.get("x-role") != "anonymous": + request.state.user = User(id=uuid4(), email="test@example.com", password_hash="x", system_role=request.headers.get("x-role", "admin")) + request.state.auth_source = request.headers.get("x-auth-source") + return await call_next(request) + + app.dependency_overrides[get_config] = lambda: SimpleNamespace() + monkeypatch.setattr(skills, "_get_user_skill_storage", lambda _: stores["alice"]) + app.include_router(skills.router) + app.state.stores = stores + return app + + +def test_manifest_download_and_changed_revision(app): + with TestClient(app) as client: + preview = client.get("/api/skills/custom/demo/export-manifest") + assert preview.status_code == 200, preview.text + manifest = preview.json() + assert manifest["can_export"] + url = "/api/skills/custom/demo/export?expected_revision=" + manifest["revision"] + response = client.get(url) + assert response.status_code == 200, response.text + assert response.headers["content-type"] == "application/zip" + assert response.headers["content-disposition"] == 'attachment; filename="demo.skill"' + assert response.headers["cache-control"] == "private, no-store" + assert response.headers["x-content-type-options"] == "nosniff" + assert int(response.headers["content-length"]) == len(response.content) + with ZipFile(BytesIO(response.content)) as archive: + assert archive.read("demo/SKILL.md").endswith(b"alice") + (app.state.stores["alice"].get_custom_skill_dir("demo") / "extra.txt").write_bytes(b"new") + stale = client.get(url) + assert stale.status_code == 409 + assert stale.json()["detail"]["code"] == "skill_changed" + assert "content-disposition" not in stale.headers + assert client.get("/api/skills/custom/demo/export").status_code == 422 + assert client.get("/api/skills/custom/demo/export?expected_revision=bad").status_code == 422 + + +@pytest.mark.parametrize("headers,status", [({"x-role": "user"}, 403), ({"x-auth-source": "pat"}, 403), ({"x-role": "anonymous"}, 401)]) +def test_auth_before_storage(app, monkeypatch, headers, status): + monkeypatch.setattr(skills, "_get_user_skill_storage", lambda _: pytest.fail("storage reached without admin")) + with TestClient(app) as client: + for suffix in ("export-manifest", "export?expected_revision=" + "a" * 64): + assert client.get("/api/skills/custom/demo/" + suffix, headers=headers).status_code == status + + +@pytest.mark.asyncio +async def test_slot_held_until_response_finishes_and_send_failure_closes(): + file = BytesIO(b"zip") + lease = service.ExportLease.acquire() + response = service.SkillExportResponse(SkillExportArchive(file, 3), "demo", lease) + second = service.ExportLease.acquire() + with pytest.raises(Exception) as error: + service.ExportLease.acquire() + assert error.value.status_code == 429 + + async def send(_): + raise OSError("client disconnected") + + async def receive(): + await asyncio.Event().wait() + + try: + with pytest.raises(ClientDisconnect): + await response({"type": "http", "asgi": {"spec_version": "2.4"}}, receive, send) + assert file.closed + third = service.ExportLease.acquire() + third.release() + finally: + second.release() + lease.release() + + +@pytest.mark.asyncio +async def test_cancel_drains_worker_and_closes_unclaimed_archive(): + started, finish = threading.Event(), threading.Event() + file = BytesIO(b"zip") + + def work(cancel_event): + started.set() + finish.wait(3) + assert cancel_event.is_set() + return SkillExportArchive(file, 3) + + task = asyncio.create_task(service.run_export_work(work)) + while not started.is_set(): + await asyncio.sleep(0.01) + task.cancel() + await asyncio.sleep(0.02) + assert not task.done() + finish.set() + with pytest.raises(asyncio.CancelledError): + await task + assert file.closed + leases = [service.ExportLease.acquire(), service.ExportLease.acquire()] + for lease in leases: + lease.release() + + +def test_same_name_stays_in_current_user_and_missing_does_not_fall_back(app, monkeypatch): + with TestClient(app) as client: + alice = client.get("/api/skills/custom/demo/export-manifest").json() + monkeypatch.setattr(skills, "_get_user_skill_storage", lambda _: app.state.stores["bob"]) + bob = client.get("/api/skills/custom/demo/export-manifest").json() + assert bob["revision"] != alice["revision"] + assert client.get("/api/skills/custom/demo/export?expected_revision=" + alice["revision"]).status_code == 409 + assert client.get("/api/skills/custom/missing/export-manifest").status_code == 404 + + +def test_busy_and_unexpected_errors_keep_safe_response(app, monkeypatch): + with TestClient(app) as client: + leases = [service.ExportLease.acquire(), service.ExportLease.acquire()] + try: + response = client.get("/api/skills/custom/demo/export-manifest") + assert response.status_code == 429 + assert response.json()["detail"]["code"] == "skill_export_busy" + finally: + for lease in leases: + lease.release() + + def failure(*args): + raise OSError("secret content at /host/private/path") + + monkeypatch.setattr(skills, "export_manifest", failure) + response = client.get("/api/skills/custom/demo/export-manifest") + assert response.status_code == 500 + assert "secret" not in response.text and "/host/" not in response.text + leases = [service.ExportLease.acquire(), service.ExportLease.acquire()] + for lease in leases: + lease.release() + + +@pytest.mark.asyncio +async def test_client_disconnect_signals_worker_and_preserves_user_context(): + from contextvars import ContextVar + + from starlette.requests import Request + + owner = ContextVar("export_test_owner", default="wrong") + token = owner.set("alice") + started = threading.Event() + disconnected = asyncio.Event() + + def work(cancel): + assert owner.get() == "alice" + started.set() + assert cancel.wait(3) + raise RuntimeError("cancelled") + + async def receive(): + await disconnected.wait() + return {"type": "http.disconnect"} + + task = asyncio.create_task(service.run_export_work(work, Request({"type": "http"}, receive))) + try: + while not started.is_set(): + await asyncio.sleep(0.01) + disconnected.set() + with pytest.raises(service.ExportClientDisconnected): + await task + leases = [service.ExportLease.acquire(), service.ExportLease.acquire()] + for lease in leases: + lease.release() + finally: + owner.reset(token) + + +def test_export_upload_roundtrip_uses_existing_scanner_and_rejects_conflict(app, monkeypatch): + """Actual public skill, production routes/scanner; only remote model decision stubbed.""" + import shutil + from pathlib import Path + + from deerflow.skills.security_scanner import ScanResult + + source = Path(__file__).resolve().parents[2] / "skills/public/data-analysis" + alice = app.state.stores["alice"] + bob = app.state.stores["bob"] + shutil.copytree(source, alice.get_custom_skill_dir("data-analysis"), ignore=shutil.ignore_patterns("__pycache__", "*.pyc")) + scanned = [] + + async def scan(content, *, executable, location, **kwargs): + scanned.append(location) + return ScanResult(decision="allow", reason="Offline remote-model stub") + + async def refresh(_): + pass + + monkeypatch.setattr("deerflow.skills.installer.scan_skill_content", scan) + monkeypatch.setattr(skills, "refresh_user_skills_system_prompt_cache_async", refresh) + with TestClient(app) as client: + manifest = client.get("/api/skills/custom/data-analysis/export-manifest").json() + archive = client.get("/api/skills/custom/data-analysis/export?expected_revision=" + manifest["revision"]) + assert archive.status_code == 200 + monkeypatch.setattr(skills, "_get_user_skill_storage", lambda _: bob) + response = client.post("/api/skills/install/upload", files={"archive": ("data-analysis.skill", archive.content, "application/zip")}) + assert response.status_code == 200, response.text + assert scanned, "Exported files must not bypass the normal import scanner" + for path in source.rglob("*"): + if path.is_file() and "__pycache__" not in path.parts and path.suffix != ".pyc": + assert (bob.get_custom_skill_dir("data-analysis") / path.relative_to(source)).read_bytes() == path.read_bytes() + conflict = client.post("/api/skills/install/upload", files={"archive": ("data-analysis.skill", archive.content, "application/zip")}) + assert conflict.status_code == 409 + + +@pytest.mark.asyncio +@pytest.mark.parametrize("suffix", ["export-manifest", "export?expected_revision=" + "a" * 64]) +async def test_disconnect_exits_router_without_asgi_error(app, monkeypatch, suffix): + started = threading.Event() + + def work(*args): + started.set() + assert args[-1].wait(3) + raise RuntimeError("worker cancelled") + + async def receive(): + while not started.is_set(): + await asyncio.sleep(0.001) + return {"type": "http.disconnect"} + + async def admin(*args, **kwargs): + pass + + monkeypatch.setattr(skills, "require_admin_user", admin) + monkeypatch.setattr(skills, "export_manifest", work) + monkeypatch.setattr(skills, "build_skill_export", work) + plain_app = FastAPI() + plain_app.dependency_overrides[get_config] = lambda: SimpleNamespace() + plain_app.include_router(skills.router) + path, _, query = ("/api/skills/custom/demo/" + suffix).partition("?") + scope = {"type": "http", "asgi": {"version": "3.0", "spec_version": "2.4"}, "http_version": "1.1", "method": "GET", "scheme": "http", "path": path, "query_string": query.encode(), "headers": []} + messages = [] + + async def send(message): + messages.append(message) + + await plain_app(scope, receive, send) + assert messages[0]["status"] == 204 + leases = [service.ExportLease.acquire(), service.ExportLease.acquire()] + for lease in leases: + lease.release() + + +@pytest.mark.asyncio +@pytest.mark.parametrize("spec_version", ["2.0", "2.4"]) +async def test_stalled_transfer_has_deadline_and_releases_archive_and_slot(monkeypatch, spec_version): + monkeypatch.setattr(service, "TRANSFER_IDLE_TIMEOUT_SECONDS", 0.02) + file = BytesIO(b"zip") + lease = service.ExportLease.acquire() + response = service.SkillExportResponse(SkillExportArchive(file, 3), "demo", lease) + body_started = asyncio.Event() + + async def send(message): + if message["type"] == "http.response.body": + body_started.set() + await asyncio.Event().wait() + + async def receive(): + await asyncio.Event().wait() + + task = asyncio.create_task(response({"type": "http", "asgi": {"spec_version": spec_version}}, receive, send)) + try: + await asyncio.wait_for(body_started.wait(), 1) + with pytest.raises(ClientDisconnect): + await asyncio.wait_for(asyncio.shield(task), 0.5) + assert file.closed + leases = [service.ExportLease.acquire(), service.ExportLease.acquire()] + for acquired in leases: + acquired.release() + finally: + task.cancel() + await asyncio.gather(task, return_exceptions=True) + + +def test_manifest_openapi_has_nested_response_contract(app): + schema = app.openapi() + response = schema["paths"]["/api/skills/custom/{skill_name}/export-manifest"]["get"]["responses"]["200"]["content"]["application/json"]["schema"] + model = schema["components"]["schemas"][response["$ref"].rsplit("/", 1)[-1]] + assert set(model["required"]) == {"skill_name", "revision", "can_export", "file_count", "directory_count", "total_bytes", "files", "requirements", "warnings", "blockers"} + for field in ("files", "warnings", "blockers"): + assert "$ref" in model["properties"][field]["items"] + assert "$ref" in model["properties"]["requirements"] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("spec_version", ["2.0", "2.4"]) +async def test_progressing_slow_transfer_can_exceed_idle_deadline(monkeypatch, spec_version): + monkeypatch.setattr(service, "TRANSFER_IDLE_TIMEOUT_SECONDS", 0.5) + content = b"x" * (6 * 1024 * 1024) + file = BytesIO(content) + response = service.SkillExportResponse(SkillExportArchive(file, len(content)), "demo", service.ExportLease.acquire()) + received = bytearray() + completed = False + + async def send(message): + nonlocal completed + if message["type"] == "http.response.body": + await asyncio.sleep(0.1) + received.extend(message.get("body", b"")) + completed = not message.get("more_body", False) + + async def receive(): + await asyncio.Event().wait() + + await response({"type": "http", "asgi": {"spec_version": spec_version}}, receive, send) + assert completed + assert received == content + assert file.closed + leases = [service.ExportLease.acquire(), service.ExportLease.acquire()] + for lease in leases: + lease.release() diff --git a/backend/tests/test_skill_export.py b/backend/tests/test_skill_export.py new file mode 100644 index 000000000..e052de5db --- /dev/null +++ b/backend/tests/test_skill_export.py @@ -0,0 +1,509 @@ +import os +import zipfile + +import pytest + +from deerflow.skills import export +from deerflow.skills.storage.local_skill_storage import LocalSkillStorage + + +@pytest.fixture +def package(tmp_path): + storage = LocalSkillStorage(host_path=str(tmp_path)) + root = tmp_path / "custom" / "sample" + root.mkdir(parents=True) + (root / "SKILL.md").write_bytes(b"---\nname: sample\ndescription: Example\n---\nHello\r\n") + return storage, root + + +def test_snapshot_revision_and_zip_roundtrip(package, tmp_path): + storage, root = package + (root / "empty").mkdir() + (root / "run.sh").write_bytes(b"#!/bin/sh\necho hello\n") + (root / "run.sh").chmod(0o751) + manifest = export.export_manifest(storage, "sample") + assert manifest["can_export"] and manifest["directory_count"] == 2 + archive = export.build_skill_export(storage, "sample", manifest["revision"]) + try: + with zipfile.ZipFile(archive.file) as z: + assert z.read("sample/SKILL.md") == (root / "SKILL.md").read_bytes() + from deerflow.skills.installer import safe_extract_skill_archive + + safe_extract_skill_archive(z, tmp_path / "imported") + assert (tmp_path / "imported/sample/empty").is_dir() + if os.name == "posix": + assert (tmp_path / "imported/sample/run.sh").stat().st_mode & 0o7777 == 0o755 + finally: + archive.close() + (root / "run.sh").chmod(0o644) + with pytest.raises(export.SkillExportError, match="changed") as error: + export.build_skill_export(storage, "sample", manifest["revision"]) + assert error.value.status == 409 + + +@pytest.mark.parametrize("kind", ["symlink", "hardlink", "nested", "reserved", "binary", "collision"]) +def test_structural_blockers(package, kind): + storage, root = package + if kind == "symlink": + (root / "link").symlink_to(root / "SKILL.md") + elif kind == "hardlink": + os.link(root / "SKILL.md", root / "hard") + elif kind == "nested": + (root / "nested").mkdir() + (root / "nested/SKILL.md").write_text("fixture") + elif kind == "reserved": + (root / "CON.txt").touch() + elif kind == "binary": + (root / "elf").write_bytes(b"\x7fELFhello") + else: + (root / "A").touch() + (root / "a").touch() + if len(list(root.iterdir())) < 3: + pytest.skip("case insensitive filesystem") + manifest = export.export_manifest(storage, "sample") + assert not manifest["can_export"] and manifest["revision"] is None + assert manifest["blockers"] + + +def test_limits_are_not_truncated(package, monkeypatch): + storage, root = package + monkeypatch.setattr(export, "MAX_ENTRIES", 1) + with pytest.raises(export.SkillExportError) as error: + export.export_manifest(storage, "sample") + assert error.value.status == 413 + + +def test_root_link_and_missing_ownership(package): + storage, root = package + root.rename(root.with_name("other")) + root.symlink_to(root.with_name("other"), target_is_directory=True) + assert export.export_manifest(storage, "sample")["blockers"][0]["code"] == "skill_export_link" + with pytest.raises(export.SkillExportError) as error: + export.export_manifest(storage, "missing") + assert error.value.status == 404 + + +def test_local_writer_waits_for_export_lock(package): + import threading + + from deerflow.skills.projection import skill_projection_read_lock + + storage, root = package + started = threading.Event() + finished = threading.Event() + + def write(): + started.set() + storage.write_custom_skill("sample", "new/resource.txt", "new") + finished.set() + + with skill_projection_read_lock(storage): + thread = threading.Thread(target=write) + thread.start() + assert started.wait(1) + assert not finished.wait(0.1) + assert not (root / "new").exists() + thread.join(2) + assert finished.is_set() + + +def test_snapshot_source_race(package, monkeypatch): + storage, root = package + original = export._walk + + def racing(*args): + result = original(*args) + (root / "SKILL.md").write_bytes((root / "SKILL.md").read_bytes() + b"changed") + return result + + monkeypatch.setattr(export, "_walk", racing) + with pytest.raises(export.SkillExportError) as error: + export.export_manifest(storage, "sample") + assert error.value.status == 409 + + +def test_zip_uses_only_snapshot(package, monkeypatch): + storage, root = package + manifest = export.export_manifest(storage, "sample") + original_bytes = (root / "SKILL.md").read_bytes() + original = export._capture + + def replace_after_capture(*args): + result = original(*args) + (root / "SKILL.md").write_bytes(b"new content") + return result + + monkeypatch.setattr(export, "_capture", replace_after_capture) + archive = export.build_skill_export(storage, "sample", manifest["revision"]) + try: + with zipfile.ZipFile(archive.file) as z: + assert z.read("sample/SKILL.md") == original_bytes + finally: + archive.close() + + +@pytest.mark.parametrize("limit", ["MAX_FILE_BYTES", "MAX_TOTAL_BYTES", "MAX_PATH_BYTES", "MAX_DEPTH"]) +def test_resource_limits(package, monkeypatch, limit): + storage, root = package + (root / "deep").mkdir() + (root / "deep/file").write_bytes(b"abc") + monkeypatch.setattr(export, limit, 1) + with pytest.raises(export.SkillExportError) as error: + export.export_manifest(storage, "sample") + assert error.value.status == 413 + + +def test_zip_limit_and_cancellation_close_files(package, monkeypatch): + import threading + + storage, root = package + manifest = export.export_manifest(storage, "sample") + files = [] + original = export.tempfile.TemporaryFile + + def track(*args, **kwargs): + file = original(*args, **kwargs) + files.append(file) + return file + + monkeypatch.setattr(export.tempfile, "TemporaryFile", track) + monkeypatch.setattr(export, "MAX_ZIP_BYTES", 1) + with pytest.raises(export.SkillExportError) as error: + export.build_skill_export(storage, "sample", manifest["revision"]) + assert error.value.status == 413 + assert all(file.closed for file in files) + event = threading.Event() + event.set() + with pytest.raises(export.SkillExportError) as error: + export.export_manifest(storage, "sample", event) + assert error.value.code == "skill_export_cancelled" + assert all(file.closed for file in files) + + +def test_diagnostics_requirements_do_not_expose_contents(package): + import json + + storage, root = package + (root / ".env").write_text("SECRET_VALUE=private-contents", encoding="utf-8") + (root / "SKILL.md").write_text("---\nname: sample\ndescription: Example\ncompatibility: Requires Python\nrequired-secrets:\n - name: API_KEY\n optional: true\n value: private-contents\n---\n", encoding="utf-8") + manifest = export.export_manifest(storage, "sample") + assert manifest["requirements"]["required_secrets"] == [{"name": "API_KEY", "optional": True}] + assert manifest["warnings"] + assert "private-contents" not in json.dumps(manifest) + + +@pytest.mark.parametrize("skill_name", ["code-documentation", "skill-creator"]) +def test_public_skill_real_install_roundtrip(tmp_path, monkeypatch, skill_name): + import shutil + from pathlib import Path + from types import SimpleNamespace + from unittest.mock import AsyncMock + + from deerflow.skills import installer + + public = Path(__file__).resolve().parents[2] / "skills/public" / skill_name + storage = LocalSkillStorage(host_path=str(tmp_path / "source")) + root = storage.get_custom_skill_dir(skill_name) + shutil.copytree(public, root, ignore=shutil.ignore_patterns("__pycache__")) + manifest = export.export_manifest(storage, skill_name) + assert manifest["can_export"], manifest["blockers"] + archive = export.build_skill_export(storage, skill_name, manifest["revision"]) + archive_path = tmp_path / "code-documentation.skill" + try: + archive_path.write_bytes(archive.file.read()) + finally: + archive.close() + # Run the real installer and deterministic scanner; substitute only the remote LLM decision. + scan = AsyncMock(return_value=SimpleNamespace(decision="allow", reason="test approval")) + monkeypatch.setattr(installer, "scan_skill_content", scan) + target = LocalSkillStorage(host_path=str(tmp_path / "target"), app_config=SimpleNamespace(skill_scan=SimpleNamespace(enabled=True))) + result = target.install_skill_from_archive(archive_path) + assert result["success"] + assert scan.called + assert export.export_manifest(target, skill_name)["revision"] == manifest["revision"] + if skill_name == "skill-creator": + import subprocess + import sys + + installed = target.get_custom_skill_dir(skill_name) + result = subprocess.run([sys.executable, str(installed / "scripts/quick_validate.py"), str(installed)], capture_output=True, text=True, timeout=10) + assert result.returncode == 0, result.stdout + result.stderr + assert "Skill is valid!" in result.stdout + + +@pytest.mark.parametrize("mode, expected", [(0o100777, 0o755), (0o107777, 0o755), (0o100640, 0o644), (0, 0o644)]) +def test_import_permissions_from_independent_zip(tmp_path, mode, expected): + import io + + from deerflow.skills.installer import safe_extract_skill_archive + + raw = io.BytesIO() + with zipfile.ZipFile(raw, "w") as archive: + info = zipfile.ZipInfo("script") + info.create_system = 3 + info.external_attr = mode << 16 + archive.writestr(info, b"#!/bin/sh\n") + raw.seek(0) + with zipfile.ZipFile(raw) as archive: + safe_extract_skill_archive(archive, tmp_path) + if os.name == "posix": + assert (tmp_path / "script").stat().st_mode & 0o7777 == expected + + +def test_cross_process_lock_timeout(package, monkeypatch): + import subprocess + import sys + + storage, root = package + lock = root.parent.parent / ".custom.projection.lock" + child = subprocess.Popen([sys.executable, "-c", 'import fcntl,sys,time; f=open(sys.argv[1],"a"); fcntl.flock(f,fcntl.LOCK_EX); print("ready",flush=True); time.sleep(10)', str(lock)], stdout=subprocess.PIPE, text=True) + try: + assert child.stdout.readline().strip() == "ready" + monkeypatch.setattr(export, "LOCK_TIMEOUT_SECONDS", 0.05) + with pytest.raises(export.SkillExportError) as error: + export.export_manifest(storage, "sample") + assert error.value.code == "skill_export_timeout" + finally: + child.terminate() + child.wait(timeout=2) + child.stdout.close() + + +def test_user_read_lock_no_projection_mutation_and_owned_only(tmp_path, monkeypatch): + from deerflow.config.paths import Paths + from deerflow.skills.projection import get_skill_projection_paths + from deerflow.skills.storage.user_scoped_skill_storage import UserScopedSkillStorage + + monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: Paths(base_dir=tmp_path)) + storage = UserScopedSkillStorage("one", host_path=str(tmp_path / "global")) + legacy = tmp_path / "global/custom/sample" + legacy.mkdir(parents=True) + (legacy / "SKILL.md").write_text("---\nname: sample\ndescription: Example\n---\n", encoding="utf-8") + with pytest.raises(export.SkillExportError) as error: + export.export_manifest(storage, "sample") + assert error.value.status == 404 + import shutil + + shutil.copytree(legacy, storage.get_custom_skill_dir("sample")) + scope = get_skill_projection_paths(storage).custom.parent + scope.mkdir(parents=True, exist_ok=True) + marker = scope / ".projection-manifest.json" + marker.write_text("unchanged", encoding="utf-8") + assert export.export_manifest(storage, "sample")["can_export"] + assert marker.read_text(encoding="utf-8") == "unchanged" + + +@pytest.mark.parametrize("user_scoped", [False, True]) +def test_temp_creation_and_cleanup_within_mutation(tmp_path, monkeypatch, user_scoped): + from contextlib import contextmanager + + from deerflow.config.paths import Paths + from deerflow.skills.storage.user_scoped_skill_storage import UserScopedSkillStorage + + monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: Paths(base_dir=tmp_path)) + storage = UserScopedSkillStorage("one", host_path=str(tmp_path)) if user_scoped else LocalSkillStorage(host_path=str(tmp_path)) + root = storage.get_custom_skill_dir("sample") + active = False + + @contextmanager + def locked(): + nonlocal active + assert not root.exists() + active = True + try: + yield + finally: + assert list(root.iterdir()) == [] + active = False + + monkeypatch.setattr(storage, "_skill_projection_mutation", locked) + from deerflow.skills.storage import local_skill_storage + + original = local_skill_storage.tempfile.NamedTemporaryFile + + @contextmanager + def failing(*args, **kwargs): + assert active + with original(*args, **kwargs) as file: + + class Broken: + name = file.name + + def write(self, content): + file.write(content) + raise OSError("simulated write failure") + + yield Broken() + + monkeypatch.setattr(local_skill_storage.tempfile, "NamedTemporaryFile", failing) + with pytest.raises(OSError): + storage.write_custom_skill("sample", "SKILL.md", "abc") + assert not active + + +def test_ancestor_symlink_is_not_followed(package, tmp_path): + storage, root = package + # A user scope ancestor is also an ownership boundary, even if custom itself is real. + custom = root.parent + custom.rename(tmp_path / "outside") + (tmp_path / "scope").symlink_to(tmp_path, target_is_directory=True) + storage.get_custom_skill_dir = lambda name: tmp_path / "scope/outside" / name + manifest = export.export_manifest(storage, "sample") + assert not manifest["can_export"] + assert manifest["blockers"][0]["code"] == "skill_export_link" + + +def test_nofollow_file_replacement_is_changed(package, monkeypatch): + storage, root = package + original = export.os.open + swapped = False + + def racing(path, flags, *args, **kwargs): + nonlocal swapped + if path == "SKILL.md" and not swapped: + swapped = True + (root / "SKILL.md").unlink() + (root / "SKILL.md").symlink_to("/etc/hosts") + return original(path, flags, *args, **kwargs) + + monkeypatch.setattr(export.os, "open", racing) + monkeypatch.setattr(export.os, "supports_dir_fd", os.supports_dir_fd | {racing}) + with pytest.raises(export.SkillExportError) as error: + export.export_manifest(storage, "sample") + assert error.value.status == 409 + + +def test_large_body_and_bounded_frontmatter(package): + storage, root = package + header = (root / "SKILL.md").read_bytes() + (root / "SKILL.md").write_bytes(header + b"body\n" * 250000) + assert export.export_manifest(storage, "sample")["can_export"] + (root / "SKILL.md").write_bytes(b"---\nname: sample\ndescription: Example\ncompatibility: " + b"x" * (1024 * 1024) + b"\n---\n") + with pytest.raises(export.SkillExportError) as error: + export.export_manifest(storage, "sample") + assert error.value.status == 413 + + +def test_invalid_utf8_in_large_body_is_blocked(package): + storage, root = package + (root / "SKILL.md").write_bytes((root / "SKILL.md").read_bytes() + b"x" * (1024 * 1024 + 1) + b"\xff") + assert not export.export_manifest(storage, "sample")["can_export"] + + +@pytest.mark.parametrize("name", ["con", "aux", "com1"]) +def test_windows_reserved_root_is_blocked(tmp_path, name): + storage = LocalSkillStorage(host_path=str(tmp_path)) + root = storage.get_custom_skill_dir(name) + root.mkdir(parents=True) + (root / "SKILL.md").write_text(f"---\nname: {name}\ndescription: Example\n---\n", encoding="utf-8") + assert not export.export_manifest(storage, name)["can_export"] + + +def test_required_secrets_normalization_and_invalid_warning(package): + storage, root = package + (root / "SKILL.md").write_text('---\nname: sample\ndescription: Example\nrequired-secrets:\n - " API_KEY "\n - name: API_KEY\n - name: " OPTIONAL_KEY "\n optional: true\n - name: []\n - 123\n---\n', encoding="utf-8") + manifest = export.export_manifest(storage, "sample") + assert manifest["requirements"]["required_secrets"] == [{"name": "API_KEY", "optional": False}, {"name": "OPTIONAL_KEY", "optional": True}] + assert any(warning["code"] == "skill_export_invalid_declaration" for warning in manifest["warnings"]) + + +def test_mtime_does_not_change_revision(package): + storage, root = package + before = export.export_manifest(storage, "sample")["revision"] + os.utime(root / "SKILL.md", (1, 1)) + assert export.export_manifest(storage, "sample")["revision"] == before + + +def test_deep_yaml_is_bounded_without_source_diagnostics(package): + storage, root = package + (root / "SKILL.md").write_text("---\nname: sample\ndescription: " + "[" * 5000 + "secret" + "]" * 5000 + "\n---\n", encoding="utf-8") + manifest = export.export_manifest(storage, "sample") + assert not manifest["can_export"] + assert "secret" not in str(manifest["blockers"]) + + +def test_invalid_unicode_nodes_still_consume_entry_budget(package, monkeypatch): + from contextlib import contextmanager + from types import SimpleNamespace + + storage, root = package + (root / "one").mkdir() + (root / "two").mkdir() + directory_inodes = {(root / "one").stat().st_ino, (root / "two").stat().st_ino} + original = export.os.scandir + + @contextmanager + def names(fd): + if os.fstat(fd).st_ino in directory_inodes: + yield iter([SimpleNamespace(name="\udcff")]) + else: + with original(fd) as iterator: + yield iterator + + monkeypatch.setattr(export.os, "scandir", names) + monkeypatch.setattr(export.os, "supports_fd", os.supports_fd | {names}) + monkeypatch.setattr(export, "MAX_ENTRIES", 4) + with pytest.raises(export.SkillExportError) as error: + export.export_manifest(storage, "sample") + assert error.value.status == 413 + + +@pytest.mark.parametrize("metadata", ["metadata: {base: &base {x: 1}, copy: *base}", "description: &text Example\nmetadata: {copy: *text}"]) +def test_yaml_alias_is_rejected_before_constructor(package, monkeypatch, metadata): + storage, root = package + (root / "SKILL.md").write_text("---\nname: sample\ndescription: Example\n" + metadata + "\n---\n", encoding="utf-8") + + def forbidden(*args, **kwargs): + raise AssertionError("YAML constructor must not run for aliases") + + monkeypatch.setattr(export, "validate_skill_frontmatter_text", forbidden) + manifest = export.export_manifest(storage, "sample") + assert not manifest["can_export"] + assert manifest["blockers"][0]["code"] == "skill_export_yaml_alias" + assert "aliases" in manifest["blockers"][0]["message"] + + +def test_yaml_merge_alias_bomb_is_blocked_without_expansion(package): + storage, root = package + levels = [" a0: &a0 {x: 1}"] + levels.extend(f" a{i}: &a{i} {{<<: [*a{i - 1}, *a{i - 1}]}}" for i in range(1, 31)) + content = "---\nname: sample\ndescription: Example\nmetadata:\n" + "\n".join(levels) + "\n---\n" + (root / "SKILL.md").write_text(content, encoding="utf-8") + manifest = export.export_manifest(storage, "sample") + assert not manifest["can_export"] + assert manifest["blockers"][0]["code"] == "skill_export_yaml_alias" + + +@pytest.mark.parametrize("kind", ["depth", "events"]) +def test_yaml_structural_budget_precedes_constructor(package, monkeypatch, kind): + storage, root = package + extra = "metadata: " + "[" * 40 + "x" + "]" * 40 if kind == "depth" else "metadata: [" + ",".join("x" for _ in range(20)) + "]" + if kind == "events": + monkeypatch.setattr(export, "MAX_YAML_EVENTS", 16) + (root / "SKILL.md").write_text("---\nname: sample\ndescription: Example\n" + extra + "\n---\n", encoding="utf-8") + + def forbidden(*args, **kwargs): + raise AssertionError("YAML constructor must not run over structural budget") + + monkeypatch.setattr(export, "validate_skill_frontmatter_text", forbidden) + manifest = export.export_manifest(storage, "sample") + assert not manifest["can_export"] + assert manifest["blockers"][0]["code"] == "skill_export_yaml_complexity" + + +def test_yaml_event_preflight_observes_cancellation(package, monkeypatch): + import threading + + storage, root = package + event = threading.Event() + original = export.yaml.parse + + def cancel_during_parse(*args, **kwargs): + for parsed in original(*args, **kwargs): + event.set() + yield parsed + + monkeypatch.setattr(export.yaml, "parse", cancel_during_parse) + with pytest.raises(export.SkillExportError) as error: + export.export_manifest(storage, "sample", event) + assert error.value.code == "skill_export_cancelled" diff --git a/frontend/AGENTS.md b/frontend/AGENTS.md index b739477eb..6fb8d372b 100644 --- a/frontend/AGENTS.md +++ b/frontend/AGENTS.md @@ -146,3 +146,9 @@ lists from the server instead of inserting those snapshots into either view. ### Delimited artifact preview CSV/TSV previews share `artifact-table-preview.tsx` between the panel and standalone viewer. Papa Parse runs only inside `delimited-preview.worker.ts`; `use-delimited-preview.ts` bounds input before transfer, cancels stale work, and enforces a five-second timeout. The parser detects the first record separator outside quoted fields and passes it explicitly to Papa Parse, so embedded newlines in an incomplete quoted field cannot corrupt newline detection. It retains at most 202 logical records and 50 columns, discarding an incomplete final record from truncated input. UI pagination displays at most 200 data rows in pages of 50. Keep the table mounted but inactive when switching to source so header/pagination state survives; changing file identity resets it. Pending `write_file` content stays in source mode until success. + +Custom skill export is admin-only and disabled in static demos. The lazy +`skill-export-dialog.tsx` must abort requests and ignore stale callbacks on close +or user/skill changes. `core/skills/export.ts` owns the revision-bound Blob download; +HTTP 409 requires explicit preview refresh. Keep file lists paginated and diagnostics +localized. Browser handoff does not prove the file was saved to disk. diff --git a/frontend/src/components/workspace/settings/skill-export-dialog.tsx b/frontend/src/components/workspace/settings/skill-export-dialog.tsx new file mode 100644 index 000000000..91b034543 --- /dev/null +++ b/frontend/src/components/workspace/settings/skill-export-dialog.tsx @@ -0,0 +1,317 @@ +"use client"; + +import { DownloadIcon, FileArchiveIcon, LoaderIcon } from "lucide-react"; +import { useCallback, useEffect, useRef, useState } from "react"; + +import { Button } from "@/components/ui/button"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from "@/components/ui/dialog"; +import { useI18n } from "@/core/i18n/hooks"; +import { + downloadSkillExport, + handOffSkillDownload, + loadSkillExportManifest, + type SkillExportManifest, + SkillExportRequestError, +} from "@/core/skills/export"; + +export default function SkillExportDialog({ + name, + onClose, +}: { + name: string; + onClose: () => void; +}) { + const { t, locale } = useI18n(); + const text = t.settings.skills; + const [manifest, setManifest] = useState(null); + const [phase, setPhase] = useState< + "loading" | "ready" | "downloading" | "done" | "error" + >("loading"); + const [error, setError] = useState(null); + const [visible, setVisible] = useState(0); + const active = useRef(null); + const downloading = useRef(false); + + const load = useCallback(async () => { + active.current?.abort(); + const controller = new AbortController(); + active.current = controller; + setManifest(null); + setError(null); + setPhase("loading"); + setVisible(0); + try { + const result = await loadSkillExportManifest(name, controller.signal); + if (controller.signal.aborted || active.current !== controller) return; + setManifest(result); + setPhase("ready"); + } catch (error) { + if (controller.signal.aborted || active.current !== controller) return; + setError(error); + setPhase("error"); + } + }, [name]); + useEffect(() => { + void load(); + return () => active.current?.abort(); + }, [load]); + + const close = () => { + active.current?.abort(); + onClose(); + }; + const download = async () => { + if (!manifest?.revision || !manifest.can_export || downloading.current) + return; + downloading.current = true; + active.current?.abort(); + const controller = new AbortController(); + active.current = controller; + setError(null); + setPhase("downloading"); + try { + const blob = await downloadSkillExport( + name, + manifest.revision, + controller.signal, + ); + if (controller.signal.aborted || active.current !== controller) return; + handOffSkillDownload(blob, name); + setPhase("done"); + } catch (error) { + if (controller.signal.aborted || active.current !== controller) return; + setError(error); + setPhase("error"); + } finally { + downloading.current = false; + } + }; + const changed = + error instanceof SkillExportRequestError && error.status === 409; + const errorMessage = + error instanceof SkillExportRequestError + ? (( + { + 403: text.installAdminRequired, + 404: text.exportNotFound, + 409: text.exportChanged, + 413: text.exportLimit, + 422: text.exportBlocked, + 429: text.exportBusy, + 503: text.exportTimeout, + } as Record + )[error.status] ?? text.exportFailed) + : text.exportFailed; + const bytes = (size: number) => + size < 1024 + ? `${size} B` + : size < 1024 * 1024 + ? `${(size / 1024).toLocaleString(locale, { maximumFractionDigits: 1 })} KiB` + : `${(size / (1024 * 1024)).toLocaleString(locale, { maximumFractionDigits: 1 })} MiB`; + + return ( + { + if (!open) close(); + }} + > + + + + + {text.exportTitle} + + {text.exportDescription} + +
+
+ {name}.skill +
+ {phase === "loading" && ( +

+ + {text.exportLoading} +

+ )} + {manifest && ( + <> +
+ {[ + [text.exportFiles, manifest.file_count], + [text.exportDirectories, manifest.directory_count], + [text.exportSize, bytes(manifest.total_bytes)], + ].map(([label, value]) => ( +
+
{label}
+
{value}
+
+ ))} +
+
+ + {text.exportContents} + +
    + {manifest.files.slice(visible, visible + 50).map((file) => ( +
  • + + {file.path === "." ? name : file.path} + {file.type === "directory" ? "/" : ""} + + + {file.type === "file" ? bytes(file.size) : "—"} + +
    • + ))} +
+ {visible > 0 && ( + + )} + {manifest.files.length > visible + 50 && ( + + )} +
+
+

{text.exportRequirements}

+
+
+
+ {text.exportCompatibility} +
+
+ {manifest.requirements.compatibility ?? + text.exportUndeclared} +
+
+
+
+ {text.exportTools} +
+
+ {manifest.requirements.allowed_tools?.join(", ") ?? + text.exportUndeclared} +
+
+
+
+ {text.exportSecrets} +
+
+ {manifest.requirements.required_secrets + ?.map( + (secret) => + `${secret.name} (${secret.optional ? text.exportOptional : text.exportRequired})`, + ) + .join(", ") ?? text.exportUndeclared} +
+
+
+
+ {manifest.warnings.length > 0 && ( +
+

{text.exportWarnings}

+

+ {text.exportWarningDescription} +

+
    + {manifest.warnings.map((warning, i) => ( +
  • + {warning.path ? `${warning.path}: ` : ""} + {text.exportNotices[warning.code] ?? warning.message} +
    • + ))} +
+
+ )} + {!manifest.can_export && ( +
+

{text.exportBlocked}

+
    + {manifest.blockers.map((blocker, i) => ( +
  • + {blocker.path ? `${blocker.path}: ` : ""} + {text.exportNotices[blocker.code] ?? blocker.message} +
    • + ))} +
+
+ )} +

+ {text.exportScope} +

+ + )} + {error !== null && ( +

+ {errorMessage} +

+ )} + {phase === "done" && ( +

+ {text.exportHandedOff} +

+ )} +
+ + + {(phase === "error" || (manifest && !manifest.can_export)) && ( + + )} + {manifest?.can_export && ( + + )} + + + + ); +} diff --git a/frontend/src/components/workspace/settings/skill-settings-page.tsx b/frontend/src/components/workspace/settings/skill-settings-page.tsx index 95ab1eb4f..5a3a80c35 100644 --- a/frontend/src/components/workspace/settings/skill-settings-page.tsx +++ b/frontend/src/components/workspace/settings/skill-settings-page.tsx @@ -1,6 +1,12 @@ "use client"; -import { LoaderIcon, SparklesIcon, UploadIcon } from "lucide-react"; +import { + DownloadIcon, + LoaderIcon, + SparklesIcon, + UploadIcon, +} from "lucide-react"; +import dynamic from "next/dynamic"; import { useRouter } from "next/navigation"; import { type ChangeEvent, useMemo, useRef, useState } from "react"; import { toast } from "sonner"; @@ -40,6 +46,10 @@ import { env } from "@/env"; import { SettingsSection } from "./settings-section"; +const SkillExportDialog = dynamic(() => import("./skill-export-dialog"), { + ssr: false, +}); + export function SkillSettingsPage({ onClose }: { onClose?: () => void } = {}) { const { t } = useI18n(); const { skills, isLoading, error } = useSkills(); @@ -78,6 +88,7 @@ function SkillSettingsList({ const router = useRouter(); const { user } = useAuth(); const isAdmin = user?.system_role === "admin"; + const [exportName, setExportName] = useState(null); const [filter, setFilter] = useState("public"); const { mutate: enableSkill } = useEnableSkill(); const fileInputRef = useRef(null); @@ -145,6 +156,15 @@ function SkillSettingsList({ }; return (
+ {exportName && + isAdmin && + env.NEXT_PUBLIC_STATIC_WEBSITE_ONLY !== "true" && ( + setExportName(null)} + /> + )}
@@ -201,6 +221,18 @@ function SkillSettingsList({ + {isAdmin && skill.category === "custom" && ( + + )} ; + exportSkill: string; + exportTitle: string; + exportDescription: string; + exportLoading: string; + exportFiles: string; + exportDirectories: string; + exportSize: string; + exportContents: string; + exportMore: string; + exportRequirements: string; + exportCompatibility: string; + exportTools: string; + exportSecrets: string; + exportOptional: string; + exportRequired: string; + exportUndeclared: string; + exportScope: string; + exportWarnings: string; + exportWarningDescription: string; + exportBlocked: string; + exportDownload: string; + exportDownloading: string; + exportHandedOff: string; + exportChanged: string; + exportRefresh: string; + exportFailed: string; + exportBusy: string; + exportTimeout: string; + exportLimit: string; + exportNotFound: string; title: string; description: string; createSkill: string; diff --git a/frontend/src/core/i18n/locales/zh-CN.ts b/frontend/src/core/i18n/locales/zh-CN.ts index 7794f9e0e..1010e617b 100644 --- a/frontend/src/core/i18n/locales/zh-CN.ts +++ b/frontend/src/core/i18n/locales/zh-CN.ts @@ -1270,6 +1270,59 @@ export const zhCN: Translations = { }, }, skills: { + exportPrevious: "上 50 项", + exportNotices: { + skill_export_yaml_alias: + "导出暂不支持 YAML 别名，请在 SKILL.md 中改为明确的值。", + skill_export_yaml_complexity: + "YAML 声明的嵌套层级或结构复杂度超出导出限制。", + skill_export_invalid_declaration: + "已忽略格式无效的凭据声明，请检查 SKILL.md。", + skill_export_link: "外链文件或目录暂不支持导出。", + skill_export_unsupported_node: + "仅支持普通文件和目录；硬链接和特殊文件无法导出。", + skill_export_invalid_path: "此路径不符合跨平台要求，或与其他路径重名。", + skill_export_nested_skill: "安装器不接受嵌套的 SKILL.md 文件。", + skill_export_executable_binary: "安装器不接受可执行二进制文件。", + skill_export_invalid_frontmatter: + "SKILL.md 的声明必须有效，且名称须与技能目录一致。", + skill_export_sensitive_filename: + "此文件名可能对应本地凭据或代码仓库元数据。", + skill_export_platform_declarations: + "请在目标环境重新配置已声明的工具和凭据。", + }, + exportSkill: "导出", + exportTitle: "导出技能", + exportDescription: "将当前已保存的技能下载为 .skill 文件。", + exportLoading: "正在准备文件清单…", + exportFiles: "文件", + exportDirectories: "目录", + exportSize: "未压缩体积", + exportContents: "包内文件", + exportMore: "下 50 项", + exportRequirements: "已声明的环境要求", + exportCompatibility: "运行环境", + exportTools: "允许的工具", + exportSecrets: "凭据名称", + exportOptional: "可选", + exportRequired: "必需", + exportUndeclared: "未声明", + exportScope: + "包含此技能目录内的全部文件。账号配置、对话和目录外的历史不会导出；目标环境需重新配置工具与凭据。", + exportWarnings: "请检查包内文件", + exportWarningDescription: + "以下提示来自文件名和声明。写在包内文件中的秘密也会原样导出；此操作不进行安全扫描。", + exportBlocked: "此技能包暂时无法导出", + exportDownload: "下载 .skill", + exportDownloading: "正在准备下载…", + exportHandedOff: "文件已交给浏览器下载。", + exportChanged: "技能已修改，请刷新文件清单后下载。", + exportRefresh: "刷新文件清单", + exportFailed: "导出失败，请重试。", + exportBusy: "当前导出任务已满，请稍后重试。", + exportTimeout: "准备技能包超时，请稍后重试。", + exportLimit: "技能包超出导出的大小、数量或路径限制。", + exportNotFound: "此自定义技能已不存在，请刷新技能列表。", title: "技能", description: "管理 Agent Skill 配置和启用状态。", createSkill: "新建技能", diff --git a/frontend/src/core/skills/export.ts b/frontend/src/core/skills/export.ts new file mode 100644 index 000000000..4940a0436 --- /dev/null +++ b/frontend/src/core/skills/export.ts @@ -0,0 +1,110 @@ +import { fetch } from "@/core/api/fetcher"; +import { getBackendBaseURL } from "@/core/config"; + +export interface SkillExportNotice { + code: string; + message: string; + path?: string; +} +export interface SkillExportManifest { + skill_name: string; + revision: string | null; + can_export: boolean; + file_count: number; + directory_count: number; + total_bytes: number; + files: { + path: string; + type: "file" | "directory"; + size: number; + executable: boolean; + }[]; + requirements: { + compatibility: string | null; + allowed_tools: string[] | null; + required_secrets: { name: string; optional: boolean }[] | null; + }; + warnings: SkillExportNotice[]; + blockers: SkillExportNotice[]; +} +export class SkillExportRequestError extends Error { + constructor( + readonly status: number, + readonly code: string, + message: string, + ) { + super(message); + this.name = "SkillExportRequestError"; + } +} +async function exportRequest( + path: string, + signal: AbortSignal, +): Promise { + const response = await fetch( + `${getBackendBaseURL()}/api/skills/custom/${path}`, + { signal, cache: "no-store" }, + ); + if (!response.ok) { + const data = (await response.json().catch(() => ({}))) as { + detail?: { code?: string; message?: string } | string; + }; + const detail = data.detail; + throw new SkillExportRequestError( + response.status, + typeof detail === "object" + ? (detail.code ?? "skill_export_failed") + : "skill_export_failed", + typeof detail === "string" + ? detail + : (detail?.message ?? "Could not export this skill."), + ); + } + return response; +} +export async function loadSkillExportManifest( + name: string, + signal: AbortSignal, +): Promise { + const response = await exportRequest( + `${encodeURIComponent(name)}/export-manifest`, + signal, + ); + return response.json() as Promise; +} +export async function downloadSkillExport( + name: string, + revision: string, + signal: AbortSignal, +): Promise { + const response = await exportRequest( + `${encodeURIComponent(name)}/export?expected_revision=${encodeURIComponent(revision)}`, + signal, + ); + if ( + response.headers.get("content-type")?.split(";")[0] !== "application/zip" + ) { + throw new SkillExportRequestError( + 502, + "skill_export_failed", + "The server did not return a skill archive.", + ); + } + return response.blob(); +} + +export function handOffSkillDownload(blob: Blob, name: string): void { + const url = URL.createObjectURL(blob); + const link = document.createElement("a"); + link.href = url; + link.download = `${name}.skill`; + try { + document.body.append(link); + link.click(); + } finally { + link.remove(); + // Allow the browser to consume the URL before releasing it, even if the + // dialog unmounts as soon as download is handed off. + setTimeout(() => URL.revokeObjectURL(url), 30_000); + } +} diff --git a/frontend/tests/e2e/skill-export.spec.ts b/frontend/tests/e2e/skill-export.spec.ts new file mode 100644 index 000000000..7a8196c4b --- /dev/null +++ b/frontend/tests/e2e/skill-export.spec.ts @@ -0,0 +1,144 @@ +import { expect, test } from "@playwright/test"; + +import { mockLangGraphAPI } from "./utils/mock-api"; + +const preview = { + skill_name: "demo", + revision: "a".repeat(64), + can_export: true, + file_count: 1, + directory_count: 1, + total_bytes: 20, + files: [{ path: "SKILL.md", type: "file", size: 20, executable: false }], + requirements: { + compatibility: "Python 3.12", + allowed_tools: ["bash"], + required_secrets: [{ name: "DEMO_TOKEN", optional: false }], + }, + warnings: [], + blockers: [], +}; + +test("custom export previews, handles stale content and downloads only after refresh", async ({ + page, +}) => { + mockLangGraphAPI(page, { + skills: [ + { + name: "demo", + description: "Synthetic UI fixture", + category: "custom", + enabled: false, + }, + { + name: "public-demo", + description: "Public fixture", + category: "public", + }, + ], + }); + let previews = 0, + downloads = 0; + await page.route("**/api/skills/custom/demo/export-manifest", (route) => { + previews++; + return route.fulfill({ json: preview }); + }); + await page.route("**/api/skills/custom/demo/export?*", (route) => { + downloads++; + expect( + new URL(route.request().url()).searchParams.get("expected_revision"), + ).toBe(preview.revision); + return downloads === 1 + ? route.fulfill({ + status: 409, + json: { detail: { code: "skill_changed", message: "Changed" } }, + }) + : route.fulfill({ + contentType: "application/zip", + body: Buffer.from("synthetic transport fixture"), + }); + }); + await page.goto("/workspace/chats/new?settings=skills"); + await expect( + page.getByRole("button", { name: "Export public-demo" }), + ).toHaveCount(0); + await page.getByRole("tab", { name: "Custom", exact: true }).click(); + await page.getByRole("button", { name: "Export demo", exact: true }).click(); + const dialog = page.getByRole("dialog", { + name: "Export skill", + exact: true, + }); + await expect(dialog.getByText("DEMO_TOKEN (required)")).toBeVisible(); + await dialog.getByRole("button", { name: "Download .skill" }).click(); + await expect( + dialog.getByText( + "The skill changed. Refresh the file list before downloading.", + ), + ).toBeVisible(); + await expect( + dialog.getByRole("button", { name: "Download .skill" }), + ).toBeDisabled(); + const beforeRefresh = previews; + await dialog.getByRole("button", { name: "Refresh file list" }).click(); + await expect( + dialog.getByRole("button", { name: "Download .skill" }), + ).toBeEnabled(); + const download = page.waitForEvent("download"); + await dialog.getByRole("button", { name: "Download .skill" }).click(); + expect((await download).suggestedFilename()).toBe("demo.skill"); + await expect( + dialog.getByText("File handed to your browser for download."), + ).toBeVisible(); + expect(previews).toBe(beforeRefresh + 1); + expect(downloads).toBe(2); + await page.keyboard.press("Escape"); + await expect(dialog).toBeHidden(); + await expect( + page.getByRole("dialog", { name: "Settings", exact: true }), + ).toBeVisible(); +}); + +test("mobile manifest blockers are readable and cannot download", async ({ + page, +}) => { + await page.setViewportSize({ width: 390, height: 844 }); + mockLangGraphAPI(page, { + skills: [ + { name: "demo", description: "Synthetic UI fixture", category: "custom" }, + ], + }); + await page.route("**/api/skills/custom/demo/export-manifest", (route) => + route.fulfill({ + json: { + ...preview, + revision: null, + can_export: false, + blockers: [ + { + code: "skill_export_link", + message: "Linked files or directories cannot be exported.", + path: "scripts/linked", + }, + ], + }, + }), + ); + await page.goto("/workspace/chats/new?settings=skills"); + await page.getByRole("tab", { name: "Custom", exact: true }).click(); + await page.getByRole("button", { name: "Export demo", exact: true }).click(); + const dialog = page.getByRole("dialog", { + name: "Export skill", + exact: true, + }); + await expect( + dialog.getByText("This package cannot be exported"), + ).toBeVisible(); + await expect( + dialog.getByRole("button", { name: "Download .skill" }), + ).toHaveCount(0); + expect( + await page.evaluate( + () => document.documentElement.scrollWidth <= innerWidth, + ), + ).toBe(true); +}); diff --git a/frontend/tests/unit/components/workspace/settings/skill-export-dialog.dom.test.tsx b/frontend/tests/unit/components/workspace/settings/skill-export-dialog.dom.test.tsx new file mode 100644 index 000000000..e49e7f83c --- /dev/null +++ b/frontend/tests/unit/components/workspace/settings/skill-export-dialog.dom.test.tsx @@ -0,0 +1,129 @@ +import { afterEach, beforeEach, describe, expect, it, rs } from "@rstest/core"; +import { + act, + cleanup, + fireEvent, + render, + screen, + waitFor, +} from "@testing-library/react"; + +const mocks = rs.hoisted(() => ({ + load: rs.fn(), + download: rs.fn(), + handoff: rs.fn(), +})); +rs.mock("@/core/skills/export", () => ({ + loadSkillExportManifest: mocks.load, + downloadSkillExport: mocks.download, + handOffSkillDownload: mocks.handoff, + SkillExportRequestError: class extends Error { + constructor( + readonly status: number, + readonly code: string, + message: string, + ) { + super(message); + } + }, +})); +rs.mock("@/core/i18n/hooks", () => ({ + useI18n: () => ({ locale: "en-US", t: enUS }), +})); +import SkillExportDialog from "@/components/workspace/settings/skill-export-dialog"; +import { enUS } from "@/core/i18n/locales/en-US"; +import { + SkillExportRequestError, + type SkillExportManifest, +} from "@/core/skills/export"; + +const manifest: SkillExportManifest = { + skill_name: "demo", + revision: "a".repeat(64), + can_export: true, + file_count: 51, + directory_count: 1, + total_bytes: 51, + files: Array.from({ length: 51 }, (_, i) => ({ + path: `file-${i}.txt`, + type: "file", + size: 1, + executable: false, + })), + requirements: { + compatibility: null, + allowed_tools: null, + required_secrets: [{ name: "DEMO_KEY", optional: true }], + }, + warnings: [], + blockers: [], +}; +beforeEach(() => { + mocks.load.mockReset().mockResolvedValue(manifest); + mocks.download.mockReset(); + mocks.handoff.mockReset(); +}); +afterEach(cleanup); + +describe("export dialog lifecycle", () => { + it("pages the file list and distinguishes undeclared dependencies", async () => { + render(); + await screen.findByText("DEMO_KEY (optional)"); + expect(screen.getAllByText("Not declared")).toHaveLength(2); + expect(screen.queryByText("file-50.txt")).toBeNull(); + fireEvent.click(screen.getByText("Next 50 files")); + expect(screen.getByText("file-50.txt")).toBeTruthy(); + expect(screen.queryByText("file-0.txt")).toBeNull(); + }); + it("requires refreshed preview after a conflict", async () => { + mocks.download.mockRejectedValue( + new SkillExportRequestError(409, "skill_changed", "changed"), + ); + render(); + fireEvent.click( + await screen.findByRole("button", { name: "Download .skill" }), + ); + await screen.findByText( + "The skill changed. Refresh the file list before downloading.", + ); + expect( + screen + .getByRole("button", { + name: "Download .skill", + }) + .hasAttribute("disabled"), + ).toBe(true); + expect(mocks.handoff).not.toHaveBeenCalled(); + fireEvent.click(screen.getByRole("button", { name: "Refresh file list" })); + await waitFor(() => expect(mocks.load).toHaveBeenCalledTimes(2)); + }); + it("ignores a download that finishes after unmount even if fetch ignores abort", async () => { + let resolve!: (blob: Blob) => void; + mocks.download.mockImplementation( + () => + new Promise((r) => { + resolve = r; + }), + ); + const view = render(); + fireEvent.click( + await screen.findByRole("button", { name: "Download .skill" }), + ); + const signal = mocks.download.mock.calls[0]![2] as AbortSignal; + view.unmount(); + expect(signal.aborted).toBe(true); + await act(async () => { + resolve(new Blob(["zip"])); + }); + expect(mocks.handoff).not.toHaveBeenCalled(); + }); + it("announces handoff only after the complete blob arrives", async () => { + mocks.download.mockResolvedValue(new Blob(["zip"])); + render(); + fireEvent.click( + await screen.findByRole("button", { name: "Download .skill" }), + ); + await screen.findByText("File handed to your browser for download."); + expect(mocks.handoff).toHaveBeenCalledTimes(1); + }); +}); diff --git a/frontend/tests/unit/core/skills/export.test.ts b/frontend/tests/unit/core/skills/export.test.ts new file mode 100644 index 000000000..a509f844b --- /dev/null +++ b/frontend/tests/unit/core/skills/export.test.ts @@ -0,0 +1,62 @@ +import { beforeEach, describe, expect, it, rs } from "@rstest/core"; + +const request = rs.hoisted(() => rs.fn()); +rs.mock("@/core/api/fetcher", () => ({ fetch: request })); +rs.mock("@/core/config", () => ({ getBackendBaseURL: () => "" })); + +import { + downloadSkillExport, + loadSkillExportManifest, + SkillExportRequestError, +} from "@/core/skills/export"; + +describe("custom skill export requests", () => { + beforeEach(() => request.mockReset()); + it("binds download to the exact preview and forwards cancellation", async () => { + request.mockResolvedValue( + new Response("zip", { headers: { "content-type": "application/zip" } }), + ); + const signal = new AbortController().signal; + const blob = await downloadSkillExport("demo", "a".repeat(64), signal); + expect(await blob.text()).toBe("zip"); + expect(request).toHaveBeenCalledWith( + "/api/skills/custom/demo/export?expected_revision=" + "a".repeat(64), + { signal, cache: "no-store" }, + ); + }); + it("surfaces a changed preview without retrying", async () => { + request.mockResolvedValue( + new Response( + JSON.stringify({ + detail: { code: "skill_changed", message: "Refresh preview" }, + }), + { status: 409 }, + ), + ); + await expect( + downloadSkillExport("demo", "a".repeat(64), new AbortController().signal), + ).rejects.toMatchObject({ status: 409, code: "skill_changed" }); + expect(request).toHaveBeenCalledTimes(1); + }); + it("preserves safe server errors for preview", async () => { + request.mockResolvedValue( + new Response( + JSON.stringify({ + detail: { code: "skill_export_busy", message: "Busy" }, + }), + { status: 429 }, + ), + ); + await expect( + loadSkillExportManifest("demo", new AbortController().signal), + ).rejects.toBeInstanceOf(SkillExportRequestError); + }); + it("does not hand an HTML response to the browser as a skill", async () => { + request.mockResolvedValue( + new Response("", { headers: { "content-type": "text/html" } }), + ); + await expect( + downloadSkillExport("demo", "a".repeat(64), new AbortController().signal), + ).rejects.toThrow(); + }); +});