mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-09 13:39:26 +00:00
feat(skills): install local skill archives (#5039)
* feat(skills): install local skill archives Signed-off-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> * fix(skills): enforce upload limits before parsing * fix(nginx): scope skill upload limit to upload route * fix(nginx): harden skill upload proxy handling * fix(skills): improve archive upload feedback * fix(skills): address upload review polish --------- Signed-off-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> Co-authored-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com>
This commit is contained in:
parent
c17aa8b98f
commit
72ba661b84
6
.github/workflows/chart.yaml
vendored
6
.github/workflows/chart.yaml
vendored
@ -22,6 +22,7 @@ on:
|
||||
- ".github/workflows/chart.yaml"
|
||||
- "scripts/check_config_version.sh"
|
||||
- "scripts/check_chart_sandbox_service.sh"
|
||||
- "scripts/check_chart_skill_upload_size.sh"
|
||||
|
||||
jobs:
|
||||
validate-chart:
|
||||
@ -49,6 +50,11 @@ jobs:
|
||||
- name: Validate sandbox Service-type gating
|
||||
run: bash scripts/check_chart_sandbox_service.sh
|
||||
|
||||
# Keep the rendered Ingress aligned with the Gateway's .skill upload
|
||||
# size, streaming, and long-running validation requirements.
|
||||
- name: Validate skill upload ingress policy
|
||||
run: bash scripts/check_chart_skill_upload_size.sh
|
||||
|
||||
# The chart's `config:` block embeds a config_version that must not fall
|
||||
# behind config.example.yaml. A stale version is silent in-cluster (the
|
||||
# image ships no example to compare against, so _check_config_version
|
||||
|
||||
@ -48,7 +48,7 @@ owner-scoped assistant version selection remains enabled.
|
||||
| **Console** (`/api/console`) | Read-only cross-thread observability for the current user (the data layer for an operations dashboard or external monitoring): `GET /stats` - headline counters (runs/threads/agents/tokens/cost); `GET /runs` - paginated run history joined with thread titles (per-run cost); `GET /usage` - zero-filled daily token series + per-model breakdown with spend. Queries `runs`/`threads_meta` directly as a reporting layer (no new `RunStore` methods); requires a SQL database backend — returns 503 on `database.backend: memory`. Real-cost estimation reads optional `models[*].pricing` (`currency`, `input_per_million`, `output_per_million`, `input_cache_hit_per_million`; `ModelConfig` is `extra="allow"`, so no schema change) and prices each run from its `token_usage_by_model` input/output split. Pricing is **cache-aware**: `RunJournal` accumulates prompt-cache hits from `usage_metadata.input_token_details.cache_read` into a sparse `cache_read_tokens` bucket key (also threaded through `SubagentTokenCollector` → `record_external_llm_usage_records`), and cache-hit input tokens are billed at `input_cache_hit_per_million` (omitted → billed at the miss price, a conservative upper bound). All priced models must use one currency; mixed currencies disable cost reporting and leave cost/currency fields null instead of producing invalid aggregates. Legacy rows fall back to run-level totals at `model_name`; unpriced models yield `cost: null` and cost fields are null when no pricing is configured |
|
||||
| **MCP** (`/api/mcp`) | `GET /config` - get config; `PUT /config` - replace the full config with whole-payload stdio validation; `PATCH /config` - toggle one server while preserving the raw extensions config and validating only an enabled target; both writes reload config and reset the process-local MCP cache |
|
||||
| **MCP Tasks** (`/api/threads/{id}/mcp-tasks`) | `GET /` - current user's durable tasks for one owned thread; `GET /{task_id}` - bounded result/input/status-error/cancellation-error detail, including cancellation attempt count, without remote task IDs or driver configuration |
|
||||
| **Skills** (`/api/skills`) | `GET /` - list skills; `GET /{name}` - details; `PUT /{name}` - update enabled; `POST /install` - install from .skill archive (accepts standard optional frontmatter like `version`, `author`, `compatibility`); `POST /reload` - admin-only process-local prompt-cache invalidation after trusted external filesystem changes |
|
||||
| **Skills** (`/api/skills`) | `GET /` - list; `GET /{name}` - inspect; `PUT /{name}` - toggle; `POST /install` - install a thread-local .skill archive; `POST /install/upload` - admin-only multipart, authorized before parsing and capped at a 100 MiB file plus 1 MiB framing; `POST /reload` - invalidate process-local cache after trusted filesystem changes |
|
||||
| **Subagents** (`/api/subagents`) | Admin managed-worker CRUD and listing. |
|
||||
| **Integrations** (`/api/integrations`) | `GET /lark/status` - inspect managed Lark/Feishu CLI integration state, including `sandbox_runtime_mode` / `sandbox_runtime_ready` (whether `lark-cli` will actually be present in the sandbox at chat time); `POST /lark/install` - admin-only install of the official `lark-*` managed skill pack; `POST /lark/config/start` and `/lark/config/complete` - internal first-time Lark connection setup; `POST /lark/config/credentials` - atomically switch the caller's per-user Lark app after validating the new `app_id`/`app_secret` through the official CLI's live tenant-token probe, revoke/remove the previous OAuth tokens, and restore the prior credential tree if the switch fails; `POST /lark/auth/start` and `/lark/auth/complete` - browser device-flow user authorization without terminal access, with optional `domains` / exact `scope` for incremental permission grants. Config and auth flows carry a server-issued, per-user generation persisted under the credential lock; a rejected direct switch leaves the current generation unchanged, stale completions return 409, and browser re-registration uses the same token-clearing/revocation transaction as direct credential switches. |
|
||||
| **Memory** (`/api/memory`) | `GET /` - memory data; `POST /reload` - force reload; `GET /config` - config; `GET /status` - config + data |
|
||||
|
||||
@ -1,11 +1,14 @@
|
||||
import asyncio
|
||||
import logging
|
||||
import tempfile
|
||||
from collections.abc import AsyncGenerator
|
||||
from pathlib import Path
|
||||
from typing import Literal
|
||||
from typing import BinaryIO, Literal
|
||||
|
||||
from fastapi import APIRouter, Depends, HTTPException, Request
|
||||
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
|
||||
@ -39,6 +42,33 @@ logger = logging.getLogger(__name__)
|
||||
router = APIRouter(prefix="/api", tags=["skills"])
|
||||
|
||||
_ADMIN_REQUIRED_DETAIL = "Admin privileges required to manage skills."
|
||||
_MAX_SKILL_ARCHIVE_UPLOAD_BYTES = 100 * 1024 * 1024
|
||||
_MAX_SKILL_ARCHIVE_MULTIPART_OVERHEAD_BYTES = 1024 * 1024
|
||||
_UPLOAD_COPY_CHUNK_BYTES = 1024 * 1024
|
||||
|
||||
|
||||
class _SkillArchiveUploadTooLargeError(MultiPartException):
|
||||
"""Abort multipart parsing while Starlette can still close spool files."""
|
||||
|
||||
|
||||
class _BoundedSkillArchiveMultiPartParser(MultiPartParser):
|
||||
"""Apply a byte limit to file parts before Starlette writes them to disk."""
|
||||
|
||||
def __init__(self, headers: Headers, stream: AsyncGenerator[bytes, None], *, max_file_bytes: int) -> None:
|
||||
super().__init__(headers, stream, max_files=1, max_fields=0)
|
||||
self._max_file_bytes = max_file_bytes
|
||||
self._current_file_bytes = 0
|
||||
|
||||
def on_part_begin(self) -> None:
|
||||
super().on_part_begin()
|
||||
self._current_file_bytes = 0
|
||||
|
||||
def on_part_data(self, data: bytes, start: int, end: int) -> None:
|
||||
if self._current_part.file is not None:
|
||||
self._current_file_bytes += end - start
|
||||
if self._current_file_bytes > self._max_file_bytes:
|
||||
raise _SkillArchiveUploadTooLargeError(_skill_archive_upload_limit_message())
|
||||
super().on_part_data(data, start, end)
|
||||
|
||||
|
||||
class SkillResponse(BaseModel):
|
||||
@ -148,6 +178,92 @@ def _get_user_skill_storage(config: AppConfig) -> SkillStorage:
|
||||
return get_or_new_user_skill_storage(get_effective_user_id(), app_config=config)
|
||||
|
||||
|
||||
def _copy_uploaded_skill_archive(source: BinaryIO) -> Path:
|
||||
"""Copy an uploaded archive to a bounded temporary file off the event loop."""
|
||||
destination: Path | None = None
|
||||
try:
|
||||
with tempfile.NamedTemporaryFile(prefix="deerflow-skill-", suffix=".skill", delete=False) as target:
|
||||
destination = Path(target.name)
|
||||
total = 0
|
||||
while chunk := source.read(_UPLOAD_COPY_CHUNK_BYTES):
|
||||
total += len(chunk)
|
||||
if total > _MAX_SKILL_ARCHIVE_UPLOAD_BYTES:
|
||||
raise _SkillArchiveUploadTooLargeError(_skill_archive_upload_limit_message())
|
||||
target.write(chunk)
|
||||
return destination
|
||||
except Exception:
|
||||
if destination is not None:
|
||||
destination.unlink(missing_ok=True)
|
||||
raise
|
||||
|
||||
|
||||
def _skill_archive_upload_limit_message() -> str:
|
||||
return f"Skill archive exceeds the {_MAX_SKILL_ARCHIVE_UPLOAD_BYTES // (1024 * 1024)} MiB upload limit"
|
||||
|
||||
|
||||
async def _bounded_skill_archive_request_stream(request: Request) -> AsyncGenerator[bytes, None]:
|
||||
"""Reject oversized multipart bodies while they are still being received."""
|
||||
request_limit = _MAX_SKILL_ARCHIVE_UPLOAD_BYTES + _MAX_SKILL_ARCHIVE_MULTIPART_OVERHEAD_BYTES
|
||||
content_length = request.headers.get("content-length")
|
||||
if content_length is not None:
|
||||
try:
|
||||
declared_length = int(content_length)
|
||||
except ValueError:
|
||||
declared_length = None
|
||||
if declared_length is not None and declared_length > request_limit:
|
||||
raise _SkillArchiveUploadTooLargeError(_skill_archive_upload_limit_message())
|
||||
|
||||
received = 0
|
||||
async for chunk in request.stream():
|
||||
received += len(chunk)
|
||||
if received > request_limit:
|
||||
raise _SkillArchiveUploadTooLargeError(_skill_archive_upload_limit_message())
|
||||
yield chunk
|
||||
|
||||
|
||||
async def _parse_skill_archive_form(request: Request) -> FormData:
|
||||
content_type = request.headers.get("content-type", "")
|
||||
media_type = content_type.partition(";")[0].strip().casefold()
|
||||
if media_type != "multipart/form-data":
|
||||
raise HTTPException(status_code=422, detail="Expected a multipart form upload")
|
||||
|
||||
parser = _BoundedSkillArchiveMultiPartParser(
|
||||
request.headers,
|
||||
_bounded_skill_archive_request_stream(request),
|
||||
max_file_bytes=_MAX_SKILL_ARCHIVE_UPLOAD_BYTES,
|
||||
)
|
||||
return await parser.parse()
|
||||
|
||||
|
||||
async def _install_skill_archive(archive_path: Path, config: AppConfig) -> SkillInstallResponse:
|
||||
try:
|
||||
result = await _get_user_skill_storage(config).ainstall_skill_from_archive(archive_path)
|
||||
await refresh_user_skills_system_prompt_cache_async(get_effective_user_id())
|
||||
return SkillInstallResponse(**result)
|
||||
except FileNotFoundError as e:
|
||||
raise HTTPException(status_code=404, detail=str(e)) from e
|
||||
except SkillAlreadyExistsError as e:
|
||||
raise HTTPException(status_code=409, detail=str(e)) from e
|
||||
except SkillSecurityScanError as e:
|
||||
if e.findings:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail={
|
||||
"message": str(e),
|
||||
"skill_name": e.skill_name,
|
||||
"findings": e.findings,
|
||||
},
|
||||
) from e
|
||||
raise HTTPException(status_code=400, detail=str(e)) from e
|
||||
except ValueError as e:
|
||||
raise HTTPException(status_code=400, detail=str(e)) from e
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
logger.error("Failed to install skill: %s", e, exc_info=True)
|
||||
raise HTTPException(status_code=500, detail=f"Failed to install skill: {str(e)}") from e
|
||||
|
||||
|
||||
@router.get(
|
||||
"/skills",
|
||||
response_model=SkillsListResponse,
|
||||
@ -174,31 +290,63 @@ async def install_skill(request: Request, body: SkillInstallRequest, config: App
|
||||
await require_admin_user(request, detail=_ADMIN_REQUIRED_DETAIL)
|
||||
try:
|
||||
skill_file_path = resolve_thread_virtual_path(body.thread_id, body.path)
|
||||
result = await _get_user_skill_storage(config).ainstall_skill_from_archive(skill_file_path)
|
||||
await refresh_user_skills_system_prompt_cache_async(get_effective_user_id())
|
||||
return SkillInstallResponse(**result)
|
||||
except FileNotFoundError as e:
|
||||
raise HTTPException(status_code=404, detail=str(e))
|
||||
except SkillAlreadyExistsError as e:
|
||||
raise HTTPException(status_code=409, detail=str(e))
|
||||
except SkillSecurityScanError as e:
|
||||
if e.findings:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail={
|
||||
"message": str(e),
|
||||
"skill_name": e.skill_name,
|
||||
"findings": e.findings,
|
||||
},
|
||||
)
|
||||
raise HTTPException(status_code=400, detail=str(e))
|
||||
raise HTTPException(status_code=404, detail=str(e)) from e
|
||||
except ValueError as e:
|
||||
raise HTTPException(status_code=400, detail=str(e))
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
logger.error(f"Failed to install skill: {e}", exc_info=True)
|
||||
raise HTTPException(status_code=500, detail=f"Failed to install skill: {str(e)}")
|
||||
raise HTTPException(status_code=400, detail=str(e)) from e
|
||||
return await _install_skill_archive(skill_file_path, config)
|
||||
|
||||
|
||||
@router.post(
|
||||
"/skills/install/upload",
|
||||
response_model=SkillInstallResponse,
|
||||
summary="Upload and Install Skill",
|
||||
description="Upload a local .skill archive and install it for the current user.",
|
||||
openapi_extra={
|
||||
"requestBody": {
|
||||
"required": True,
|
||||
"content": {
|
||||
"multipart/form-data": {
|
||||
"schema": {
|
||||
"type": "object",
|
||||
"required": ["archive"],
|
||||
"properties": {"archive": {"type": "string", "format": "binary"}},
|
||||
}
|
||||
}
|
||||
},
|
||||
}
|
||||
},
|
||||
)
|
||||
async def upload_and_install_skill(
|
||||
request: Request,
|
||||
config: AppConfig = Depends(get_config),
|
||||
) -> SkillInstallResponse:
|
||||
await require_admin_user(request, detail=_ADMIN_REQUIRED_DETAIL)
|
||||
|
||||
form: FormData | None = None
|
||||
temporary_path: Path | None = None
|
||||
try:
|
||||
form = await _parse_skill_archive_form(request)
|
||||
archive = form.get("archive")
|
||||
if not isinstance(archive, UploadFile):
|
||||
raise HTTPException(status_code=422, detail="Multipart field 'archive' must contain a file")
|
||||
|
||||
filename = archive.filename or ""
|
||||
if not filename.casefold().endswith(".skill"):
|
||||
raise HTTPException(status_code=400, detail="Skill archive filename must end with .skill")
|
||||
|
||||
await archive.seek(0)
|
||||
temporary_path = await asyncio.to_thread(_copy_uploaded_skill_archive, archive.file)
|
||||
return await _install_skill_archive(temporary_path, config)
|
||||
except _SkillArchiveUploadTooLargeError as e:
|
||||
raise HTTPException(status_code=413, detail=e.message) from e
|
||||
except MultiPartException as e:
|
||||
raise HTTPException(status_code=400, detail=e.message) from e
|
||||
finally:
|
||||
if form is not None:
|
||||
await form.close()
|
||||
if temporary_path is not None:
|
||||
await asyncio.to_thread(temporary_path.unlink, missing_ok=True)
|
||||
|
||||
|
||||
@router.post(
|
||||
|
||||
@ -128,3 +128,24 @@ def test_uploads_route_still_has_its_own_body_size_settings(path):
|
||||
|
||||
assert "client_max_body_size 100M;" in block
|
||||
assert "proxy_request_buffering off;" in block
|
||||
|
||||
|
||||
@pytest.mark.parametrize("path", NGINX_CONFIGS)
|
||||
def test_skills_upload_route_allows_archive_plus_multipart_framing(path):
|
||||
"""The upload route must stream archives and allow slow validation."""
|
||||
content = _read(path)
|
||||
block = _extract_location_block(content, "= /api/skills/install/upload")
|
||||
|
||||
assert "client_max_body_size 101M;" in block
|
||||
assert "proxy_request_buffering off;" in block
|
||||
assert "proxy_read_timeout 600s;" in block
|
||||
|
||||
|
||||
@pytest.mark.parametrize("path", NGINX_CONFIGS)
|
||||
def test_skills_prefix_keeps_default_request_body_policy(path):
|
||||
"""Large bodies must be allowed only on the admin upload endpoint."""
|
||||
content = _read(path)
|
||||
block = _extract_location_block(content, "/api/skills")
|
||||
|
||||
assert "client_max_body_size" not in block
|
||||
assert "proxy_request_buffering" not in block
|
||||
|
||||
@ -6,6 +6,7 @@ from io import BytesIO
|
||||
from pathlib import Path
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
from _router_auth_helpers import make_authed_test_app
|
||||
from fastapi import FastAPI
|
||||
from fastapi.testclient import TestClient
|
||||
@ -136,6 +137,157 @@ def test_install_skill_archive_runs_security_scan(monkeypatch, tmp_path):
|
||||
assert refresh_calls == [("refresh", "default")]
|
||||
|
||||
|
||||
def test_upload_skill_archive_installs_without_thread_workspace(monkeypatch, tmp_path):
|
||||
installed_paths: list[Path] = []
|
||||
refresh_calls: list[str] = []
|
||||
|
||||
class _Storage:
|
||||
async def ainstall_skill_from_archive(self, archive_path: Path) -> dict:
|
||||
installed_paths.append(archive_path)
|
||||
assert archive_path.name.endswith(".skill")
|
||||
assert archive_path.read_bytes() == b"skill archive bytes"
|
||||
return {
|
||||
"success": True,
|
||||
"skill_name": "uploaded-skill",
|
||||
"message": "Skill installed successfully",
|
||||
}
|
||||
|
||||
async def _refresh(user_id: str) -> None:
|
||||
refresh_calls.append(user_id)
|
||||
|
||||
config = SimpleNamespace()
|
||||
monkeypatch.setattr(skills_router, "_get_user_skill_storage", lambda cfg: _Storage())
|
||||
monkeypatch.setattr(skills_router, "refresh_user_skills_system_prompt_cache_async", _refresh)
|
||||
monkeypatch.setattr(skills_router, "get_effective_user_id", lambda: "default")
|
||||
|
||||
app = _make_test_app(config)
|
||||
with TestClient(app) as client:
|
||||
response = client.post(
|
||||
"/api/skills/install/upload",
|
||||
files={
|
||||
"archive": (
|
||||
"uploaded-skill.skill",
|
||||
b"skill archive bytes",
|
||||
"application/octet-stream",
|
||||
)
|
||||
},
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json()["skill_name"] == "uploaded-skill"
|
||||
assert refresh_calls == ["default"]
|
||||
assert len(installed_paths) == 1
|
||||
assert not installed_paths[0].exists()
|
||||
|
||||
|
||||
def test_upload_skill_archive_rejects_non_skill_extension(monkeypatch):
|
||||
install_called = False
|
||||
|
||||
class _Storage:
|
||||
async def ainstall_skill_from_archive(self, archive_path: Path) -> dict:
|
||||
nonlocal install_called
|
||||
install_called = True
|
||||
return {}
|
||||
|
||||
monkeypatch.setattr(skills_router, "_get_user_skill_storage", lambda cfg: _Storage())
|
||||
app = _make_test_app(SimpleNamespace())
|
||||
|
||||
with TestClient(app) as client:
|
||||
response = client.post(
|
||||
"/api/skills/install/upload",
|
||||
files={"archive": ("not-a-skill.zip", b"zip bytes", "application/zip")},
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
assert response.json()["detail"] == "Skill archive filename must end with .skill"
|
||||
assert install_called is False
|
||||
|
||||
|
||||
def test_upload_skill_archive_keeps_multipart_openapi_contract():
|
||||
app = _make_test_app(SimpleNamespace())
|
||||
|
||||
operation = app.openapi()["paths"]["/api/skills/install/upload"]["post"]
|
||||
request_body = operation["requestBody"]
|
||||
archive_schema = request_body["content"]["multipart/form-data"]["schema"]
|
||||
|
||||
assert request_body["required"] is True
|
||||
assert archive_schema["required"] == ["archive"]
|
||||
assert archive_schema["properties"]["archive"] == {"type": "string", "format": "binary"}
|
||||
|
||||
|
||||
def test_upload_skill_archive_rejects_oversized_payload(monkeypatch):
|
||||
install_called = False
|
||||
copy_called = False
|
||||
|
||||
class _Storage:
|
||||
async def ainstall_skill_from_archive(self, archive_path: Path) -> dict:
|
||||
nonlocal install_called
|
||||
install_called = True
|
||||
return {}
|
||||
|
||||
monkeypatch.setattr(skills_router, "_MAX_SKILL_ARCHIVE_UPLOAD_BYTES", 3)
|
||||
monkeypatch.setattr(skills_router, "_get_user_skill_storage", lambda cfg: _Storage())
|
||||
|
||||
def _unexpected_copy(source):
|
||||
nonlocal copy_called
|
||||
copy_called = True
|
||||
raise AssertionError("oversized upload reached the post-parse copy")
|
||||
|
||||
monkeypatch.setattr(skills_router, "_copy_uploaded_skill_archive", _unexpected_copy)
|
||||
app = _make_test_app(SimpleNamespace())
|
||||
|
||||
with TestClient(app) as client:
|
||||
response = client.post(
|
||||
"/api/skills/install/upload",
|
||||
files={"archive": ("demo.skill", b"four", "application/octet-stream")},
|
||||
)
|
||||
|
||||
assert response.status_code == 413
|
||||
assert "upload limit" in response.json()["detail"]
|
||||
assert install_called is False
|
||||
assert copy_called is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_bounded_skill_archive_stream_rejects_declared_oversize_before_read(monkeypatch):
|
||||
stream_read = False
|
||||
|
||||
class _Request:
|
||||
headers = {"content-length": "5"}
|
||||
|
||||
async def stream(self):
|
||||
nonlocal stream_read
|
||||
stream_read = True
|
||||
yield b"never-read"
|
||||
|
||||
monkeypatch.setattr(skills_router, "_MAX_SKILL_ARCHIVE_UPLOAD_BYTES", 3)
|
||||
monkeypatch.setattr(skills_router, "_MAX_SKILL_ARCHIVE_MULTIPART_OVERHEAD_BYTES", 1)
|
||||
stream = skills_router._bounded_skill_archive_request_stream(_Request())
|
||||
|
||||
with pytest.raises(skills_router._SkillArchiveUploadTooLargeError):
|
||||
await anext(stream)
|
||||
|
||||
assert stream_read is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_bounded_skill_archive_stream_rejects_chunked_oversize(monkeypatch):
|
||||
class _Request:
|
||||
headers = {}
|
||||
|
||||
async def stream(self):
|
||||
yield b"123"
|
||||
yield b"45"
|
||||
|
||||
monkeypatch.setattr(skills_router, "_MAX_SKILL_ARCHIVE_UPLOAD_BYTES", 3)
|
||||
monkeypatch.setattr(skills_router, "_MAX_SKILL_ARCHIVE_MULTIPART_OVERHEAD_BYTES", 1)
|
||||
stream = skills_router._bounded_skill_archive_request_stream(_Request())
|
||||
|
||||
assert await anext(stream) == b"123"
|
||||
with pytest.raises(skills_router._SkillArchiveUploadTooLargeError):
|
||||
await anext(stream)
|
||||
|
||||
|
||||
def test_uploaded_skill_archive_installs_sandbox_readable_tree(monkeypatch, tmp_path):
|
||||
home = tmp_path / "home"
|
||||
skills_root = tmp_path / "skills"
|
||||
|
||||
@ -81,6 +81,27 @@ def test_non_admin_is_forbidden_on_all_mutating_skills_endpoints():
|
||||
assert resp.status_code == 403, f"{method.upper()} {path} expected 403 for non-admin, got {resp.status_code}"
|
||||
|
||||
|
||||
def test_non_admin_upload_is_rejected_before_multipart_parsing(monkeypatch):
|
||||
parse_called = False
|
||||
|
||||
async def _unexpected_parse(request):
|
||||
nonlocal parse_called
|
||||
parse_called = True
|
||||
raise AssertionError("multipart parsing ran before the admin guard")
|
||||
|
||||
monkeypatch.setattr(skills_router, "_parse_skill_archive_form", _unexpected_parse)
|
||||
app = _make_app(system_role="user")
|
||||
|
||||
with TestClient(app) as client:
|
||||
response = client.post(
|
||||
"/api/skills/install/upload",
|
||||
files={"archive": ("demo.skill", b"archive bytes", "application/octet-stream")},
|
||||
)
|
||||
|
||||
assert response.status_code == 403
|
||||
assert parse_called is False
|
||||
|
||||
|
||||
def test_basic_skill_listing_stays_open_to_normal_users(monkeypatch):
|
||||
"""The basic list/detail endpoints expose only name/description and are
|
||||
needed by the normal-user UI, so they must NOT be admin-gated.
|
||||
|
||||
@ -119,6 +119,13 @@ secrets:
|
||||
# add channel tokens, search keys, etc. as needed
|
||||
```
|
||||
|
||||
The default ingress annotations permit a 100 MiB local `.skill` archive plus
|
||||
multipart framing, stream request bodies without ingress buffering, and allow
|
||||
up to 600 seconds for validation. If you replace `ingress.annotations`,
|
||||
preserve equivalent size, streaming, and response-timeout settings for your
|
||||
ingress controller or local skill uploads may fail before DeerFlow completes
|
||||
the installation.
|
||||
|
||||
Provide your model config under `config` (keep secrets as `$VAR` references —
|
||||
they resolve from the `secrets` map):
|
||||
|
||||
|
||||
@ -112,6 +112,20 @@ data:
|
||||
proxy_set_header X-Forwarded-Proto $forwarded_proto;
|
||||
}
|
||||
|
||||
# Admin-only local .skill archive upload from Settings.
|
||||
location = /api/skills/install/upload {
|
||||
proxy_pass http://gateway_upstream;
|
||||
proxy_http_version 1.1;
|
||||
proxy_set_header Host $http_host;
|
||||
proxy_set_header X-Real-IP $remote_addr;
|
||||
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
|
||||
proxy_set_header X-Forwarded-Proto $forwarded_proto;
|
||||
|
||||
client_max_body_size 101M;
|
||||
proxy_request_buffering off;
|
||||
proxy_read_timeout 600s;
|
||||
}
|
||||
|
||||
location /api/skills {
|
||||
proxy_pass http://gateway_upstream;
|
||||
proxy_http_version 1.1;
|
||||
|
||||
@ -234,7 +234,13 @@ ingress:
|
||||
enabled: true
|
||||
className: "nginx"
|
||||
host: "deer-flow.example.com"
|
||||
annotations: {}
|
||||
annotations:
|
||||
# Allows a 100 MiB .skill archive plus multipart framing at the outer ingress.
|
||||
nginx.ingress.kubernetes.io/proxy-body-size: "101m"
|
||||
# Streams request bodies to nginx instead of spooling them at the outer ingress.
|
||||
nginx.ingress.kubernetes.io/proxy-request-buffering: "off"
|
||||
# Skill validation may perform multiple sequential LLM calls.
|
||||
nginx.ingress.kubernetes.io/proxy-read-timeout: "600"
|
||||
tls:
|
||||
enabled: false
|
||||
secretName: ""
|
||||
|
||||
@ -145,6 +145,20 @@ http {
|
||||
|
||||
}
|
||||
|
||||
# Admin-only local .skill archive upload from Settings.
|
||||
location = /api/skills/install/upload {
|
||||
proxy_pass http://$gateway_upstream;
|
||||
proxy_http_version 1.1;
|
||||
proxy_set_header Host $http_host;
|
||||
proxy_set_header X-Real-IP $remote_addr;
|
||||
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
|
||||
proxy_set_header X-Forwarded-Proto $forwarded_proto;
|
||||
|
||||
client_max_body_size 101M;
|
||||
proxy_request_buffering off;
|
||||
proxy_read_timeout 600s;
|
||||
}
|
||||
|
||||
# Custom API: Skills configuration endpoint
|
||||
location /api/skills {
|
||||
proxy_pass http://$gateway_upstream;
|
||||
@ -153,7 +167,6 @@ http {
|
||||
proxy_set_header X-Real-IP $remote_addr;
|
||||
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
|
||||
proxy_set_header X-Forwarded-Proto $forwarded_proto;
|
||||
|
||||
}
|
||||
|
||||
# Custom API: Agents endpoint
|
||||
|
||||
@ -142,6 +142,22 @@ http {
|
||||
proxy_cache off;
|
||||
}
|
||||
|
||||
# Admin-only local .skill archive upload from Settings.
|
||||
location = /api/skills/install/upload {
|
||||
proxy_pass http://gateway;
|
||||
proxy_http_version 1.1;
|
||||
proxy_set_header Host $http_host;
|
||||
proxy_set_header X-Real-IP $remote_addr;
|
||||
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
|
||||
proxy_set_header X-Forwarded-Proto $scheme;
|
||||
|
||||
client_max_body_size 101M;
|
||||
proxy_request_buffering off;
|
||||
proxy_read_timeout 600s;
|
||||
proxy_buffering off;
|
||||
proxy_cache off;
|
||||
}
|
||||
|
||||
# Custom API: Skills configuration endpoint
|
||||
location /api/skills {
|
||||
proxy_pass http://gateway;
|
||||
@ -150,7 +166,6 @@ http {
|
||||
proxy_set_header X-Real-IP $remote_addr;
|
||||
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
|
||||
proxy_set_header X-Forwarded-Proto $scheme;
|
||||
|
||||
proxy_buffering off;
|
||||
proxy_cache off;
|
||||
}
|
||||
|
||||
@ -60,6 +60,12 @@
|
||||
|
||||
The chat header's context-window control is intentionally persistent: while `context_usage` is unavailable, `ContextUsageBadge` renders a gauge placeholder rather than unmounting; once data arrives, the same position shows the percentage. `useThreadTokenUsage` retains placeholder data only when the response `thread_id` still matches the active route, so same-thread refetches do not flicker and cross-thread navigation never displays the previous chat's usage.
|
||||
|
||||
Settings skill uploads reject archives larger than 100 MiB before starting the
|
||||
request and disable the hidden file input while an install is pending. The API
|
||||
client preserves structured SkillScan findings on `SkillRequestError`, and the
|
||||
settings page renders a compact file/rule/line summary so security rejections
|
||||
remain actionable; a proxy-generated 413 is mapped to the localized size error.
|
||||
|
||||
Run duration is run-scoped UI metadata even though the compatibility field `additional_kwargs.turn_duration` is repeated on historical AI messages. `core/messages/run-duration.ts` folds those copies into one display anchored after the run's last visible message group. `MessageList` owns the temporary client-side duration for a just-completed live turn until authoritative history arrives. The duration is total run wall-clock time, not per-message reasoning time; reasoning disclosure and run activity/duration are rendered separately.
|
||||
|
||||
The workspace-change card follows the same rule: it is resolved from `(threadId, runId)` alone, so every AI message of a run would render an identical copy. A run ends in more than one terminal assistant bubble whenever the model emits answer text that never gains a tool call, so `core/messages/workspace-change-anchor.ts` picks the run's last assistant bubble and `MessageListItem` renders the badge only for that anchor (#4555). Any future run-scoped display belongs in the same place — do not hang one off every message. The two anchor helpers deliberately differ in which group types they accept as a run's last position, because an anchor is only useful where the display is actually rendered: run duration is emitted by `MessageList` around every group, so it accepts any type, while the workspace-change card comes from `MessageListItem` and so restricts to `assistant`. Keep a new helper's candidate set matched to its own render site rather than unifying them.
|
||||
|
||||
@ -1,8 +1,9 @@
|
||||
"use client";
|
||||
|
||||
import { SparklesIcon } from "lucide-react";
|
||||
import { LoaderIcon, SparklesIcon, UploadIcon } from "lucide-react";
|
||||
import { useRouter } from "next/navigation";
|
||||
import { useMemo, useState } from "react";
|
||||
import { type ChangeEvent, useMemo, useRef, useState } from "react";
|
||||
import { toast } from "sonner";
|
||||
|
||||
import { Button } from "@/components/ui/button";
|
||||
import {
|
||||
@ -24,8 +25,16 @@ import { Switch } from "@/components/ui/switch";
|
||||
import { Tabs, TabsList, TabsTrigger } from "@/components/ui/tabs";
|
||||
import { useAuth } from "@/core/auth/AuthProvider";
|
||||
import { useI18n } from "@/core/i18n/hooks";
|
||||
import { SkillRequestError } from "@/core/skills/api";
|
||||
import { useEnableSkill, useSkills } from "@/core/skills/hooks";
|
||||
import {
|
||||
formatSkillSecurityFindings,
|
||||
MAX_SKILL_ARCHIVE_UPLOAD_BYTES,
|
||||
SkillRequestError,
|
||||
} from "@/core/skills/api";
|
||||
import {
|
||||
useEnableSkill,
|
||||
useSkills,
|
||||
useUploadSkillArchive,
|
||||
} from "@/core/skills/hooks";
|
||||
import type { Skill } from "@/core/skills/type";
|
||||
import { env } from "@/env";
|
||||
|
||||
@ -69,6 +78,11 @@ function SkillSettingsList({
|
||||
const isAdmin = user?.system_role === "admin";
|
||||
const [filter, setFilter] = useState<string>("public");
|
||||
const { mutate: enableSkill } = useEnableSkill();
|
||||
const fileInputRef = useRef<HTMLInputElement>(null);
|
||||
const { mutateAsync: uploadSkillArchive, isPending: isUploading } =
|
||||
useUploadSkillArchive();
|
||||
const isArchiveUploadDisabled =
|
||||
isUploading || !isAdmin || env.NEXT_PUBLIC_STATIC_WEBSITE_ONLY === "true";
|
||||
const filteredSkills = useMemo(
|
||||
() => skills.filter((skill) => skill.category === filter),
|
||||
[skills, filter],
|
||||
@ -77,18 +91,93 @@ function SkillSettingsList({
|
||||
onClose?.();
|
||||
router.push("/workspace/chats/new?mode=skill");
|
||||
};
|
||||
const handleSkillArchive = async (event: ChangeEvent<HTMLInputElement>) => {
|
||||
if (isUploading) {
|
||||
event.target.value = "";
|
||||
return;
|
||||
}
|
||||
const archive = event.target.files?.[0];
|
||||
event.target.value = "";
|
||||
if (!archive) return;
|
||||
if (!archive.name.toLowerCase().endsWith(".skill")) {
|
||||
toast.error(t.settings.skills.invalidArchive);
|
||||
return;
|
||||
}
|
||||
if (archive.size > MAX_SKILL_ARCHIVE_UPLOAD_BYTES) {
|
||||
toast.error(t.settings.skills.archiveTooLarge);
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
const result = await uploadSkillArchive(archive);
|
||||
if (result.success) {
|
||||
toast.success(result.message);
|
||||
setFilter("custom");
|
||||
} else {
|
||||
toast.error(result.message || t.settings.skills.installFailed);
|
||||
}
|
||||
} catch (error) {
|
||||
if (error instanceof SkillRequestError && error.isAdminRequired) {
|
||||
toast.error(t.settings.skills.installAdminRequired);
|
||||
} else if (error instanceof SkillRequestError && error.status === 413) {
|
||||
toast.error(t.settings.skills.archiveTooLarge);
|
||||
} else if (
|
||||
error instanceof SkillRequestError &&
|
||||
error.findings.length > 0
|
||||
) {
|
||||
toast.error(error.message, {
|
||||
description: (
|
||||
<span className="whitespace-pre-line">
|
||||
{formatSkillSecurityFindings(error.findings)}
|
||||
</span>
|
||||
),
|
||||
});
|
||||
} else {
|
||||
toast.error(
|
||||
error instanceof Error
|
||||
? error.message
|
||||
: t.settings.skills.installFailed,
|
||||
);
|
||||
}
|
||||
}
|
||||
};
|
||||
return (
|
||||
<div className="flex w-full flex-col gap-4">
|
||||
<header className="flex justify-between">
|
||||
<div className="flex gap-2">
|
||||
<Tabs defaultValue="public" onValueChange={setFilter}>
|
||||
<Tabs value={filter} onValueChange={setFilter}>
|
||||
<TabsList variant="line">
|
||||
<TabsTrigger value="public">{t.common.public}</TabsTrigger>
|
||||
<TabsTrigger value="custom">{t.common.custom}</TabsTrigger>
|
||||
</TabsList>
|
||||
</Tabs>
|
||||
</div>
|
||||
<div>
|
||||
<div className="flex gap-2">
|
||||
<input
|
||||
ref={fileInputRef}
|
||||
type="file"
|
||||
accept=".skill"
|
||||
disabled={isArchiveUploadDisabled}
|
||||
className="sr-only"
|
||||
onChange={handleSkillArchive}
|
||||
/>
|
||||
{isAdmin && (
|
||||
<Button
|
||||
size="sm"
|
||||
variant="outline"
|
||||
disabled={isArchiveUploadDisabled}
|
||||
onClick={() => fileInputRef.current?.click()}
|
||||
>
|
||||
{isUploading ? (
|
||||
<LoaderIcon className="size-4 animate-spin" />
|
||||
) : (
|
||||
<UploadIcon className="size-4" />
|
||||
)}
|
||||
{isUploading
|
||||
? t.settings.skills.installingArchive
|
||||
: t.settings.skills.installFromFile}
|
||||
</Button>
|
||||
)}
|
||||
<Button size="sm" onClick={handleCreateSkill}>
|
||||
<SparklesIcon className="size-4" />
|
||||
{t.settings.skills.createSkill}
|
||||
|
||||
@ -1221,6 +1221,11 @@ export const enUS: Translations = {
|
||||
adminRequired: "Admin privileges are required to manage agent skills.",
|
||||
installAdminRequired:
|
||||
"Admin privileges are required to install agent skills.",
|
||||
installFromFile: "Install .skill",
|
||||
installingArchive: "Installing...",
|
||||
invalidArchive: "Choose a file with the .skill extension.",
|
||||
archiveTooLarge: "The skill archive must be 100 MiB or smaller.",
|
||||
installFailed: "Failed to install the skill archive.",
|
||||
},
|
||||
notification: {
|
||||
title: "Notification",
|
||||
|
||||
@ -972,6 +972,11 @@ export interface Translations {
|
||||
emptyButton: string;
|
||||
adminRequired: string;
|
||||
installAdminRequired: string;
|
||||
installFromFile: string;
|
||||
installingArchive: string;
|
||||
invalidArchive: string;
|
||||
archiveTooLarge: string;
|
||||
installFailed: string;
|
||||
};
|
||||
notification: {
|
||||
title: string;
|
||||
|
||||
@ -1166,6 +1166,11 @@ export const zhCN: Translations = {
|
||||
emptyButton: "创建你的第一个技能",
|
||||
adminRequired: "需要管理员权限才能管理 Agent Skill。",
|
||||
installAdminRequired: "需要管理员权限才能安装 Agent Skill。",
|
||||
installFromFile: "安装 .skill",
|
||||
installingArchive: "正在安装…",
|
||||
invalidArchive: "请选择扩展名为 .skill 的文件。",
|
||||
archiveTooLarge: "技能包大小不能超过 100 MiB。",
|
||||
installFailed: "安装技能包失败。",
|
||||
},
|
||||
notification: {
|
||||
title: "通知",
|
||||
|
||||
@ -3,13 +3,38 @@ import { getBackendBaseURL } from "@/core/config";
|
||||
|
||||
import type { Skill } from "./type";
|
||||
|
||||
// Keep this in lockstep with `_MAX_SKILL_ARCHIVE_UPLOAD_BYTES` in
|
||||
// `backend/app/gateway/routers/skills.py`; nginx and Ingress allow 101 MiB so
|
||||
// multipart framing fits around the same 100 MiB archive limit.
|
||||
export const MAX_SKILL_ARCHIVE_UPLOAD_BYTES = 100 * 1024 * 1024;
|
||||
|
||||
export interface SkillSecurityFinding {
|
||||
rule_id: string;
|
||||
severity: string;
|
||||
file: string | null;
|
||||
line: number | null;
|
||||
message: string;
|
||||
remediation: string | null;
|
||||
}
|
||||
|
||||
export class SkillRequestError extends Error {
|
||||
readonly status: number;
|
||||
readonly skillName?: string;
|
||||
readonly findings: SkillSecurityFinding[];
|
||||
|
||||
constructor(status: number, message: string) {
|
||||
constructor(
|
||||
status: number,
|
||||
message: string,
|
||||
options: {
|
||||
skillName?: string;
|
||||
findings?: SkillSecurityFinding[];
|
||||
} = {},
|
||||
) {
|
||||
super(message);
|
||||
this.name = "SkillRequestError";
|
||||
this.status = status;
|
||||
this.skillName = options.skillName;
|
||||
this.findings = options.findings ?? [];
|
||||
}
|
||||
|
||||
get isAdminRequired(): boolean {
|
||||
@ -17,17 +42,90 @@ export class SkillRequestError extends Error {
|
||||
}
|
||||
}
|
||||
|
||||
async function readErrorDetail(response: Response): Promise<string> {
|
||||
interface SkillErrorDetail {
|
||||
message: string;
|
||||
skillName?: string;
|
||||
findings: SkillSecurityFinding[];
|
||||
}
|
||||
|
||||
function parseSecurityFindings(value: unknown): SkillSecurityFinding[] {
|
||||
if (!Array.isArray(value)) return [];
|
||||
return value.flatMap((candidate) => {
|
||||
if (typeof candidate !== "object" || candidate === null) return [];
|
||||
const finding = candidate as Record<string, unknown>;
|
||||
if (
|
||||
typeof finding.rule_id !== "string" ||
|
||||
typeof finding.severity !== "string" ||
|
||||
typeof finding.message !== "string"
|
||||
) {
|
||||
return [];
|
||||
}
|
||||
return [
|
||||
{
|
||||
rule_id: finding.rule_id,
|
||||
severity: finding.severity,
|
||||
file: typeof finding.file === "string" ? finding.file : null,
|
||||
line: typeof finding.line === "number" ? finding.line : null,
|
||||
message: finding.message,
|
||||
remediation:
|
||||
typeof finding.remediation === "string" ? finding.remediation : null,
|
||||
},
|
||||
];
|
||||
});
|
||||
}
|
||||
|
||||
export function formatSkillSecurityFindings(
|
||||
findings: SkillSecurityFinding[],
|
||||
): string {
|
||||
const lines = findings.slice(0, 3).map((finding) => {
|
||||
const location = finding.file
|
||||
? `${finding.file}${finding.line === null ? "" : `:${finding.line}`}`
|
||||
: finding.line === null
|
||||
? "archive"
|
||||
: `archive:${finding.line}`;
|
||||
return `${finding.severity} ${finding.rule_id} · ${location}: ${finding.message}${finding.remediation ? ` ${finding.remediation}` : ""}`;
|
||||
});
|
||||
const omittedCount = findings.length - lines.length;
|
||||
if (omittedCount > 0) {
|
||||
lines.push(`... and ${omittedCount} more`);
|
||||
}
|
||||
return lines.join("\n");
|
||||
}
|
||||
|
||||
async function readErrorDetail(response: Response): Promise<SkillErrorDetail> {
|
||||
const data = (await response.json().catch(() => ({}))) as {
|
||||
detail?: string;
|
||||
detail?:
|
||||
| string
|
||||
| {
|
||||
message?: unknown;
|
||||
skill_name?: unknown;
|
||||
findings?: unknown;
|
||||
};
|
||||
};
|
||||
if (typeof data.detail === "string") {
|
||||
return { message: data.detail, findings: [] };
|
||||
}
|
||||
if (typeof data.detail?.message === "string") {
|
||||
return {
|
||||
message: data.detail.message,
|
||||
skillName:
|
||||
typeof data.detail.skill_name === "string"
|
||||
? data.detail.skill_name
|
||||
: undefined,
|
||||
findings: parseSecurityFindings(data.detail.findings),
|
||||
};
|
||||
}
|
||||
return {
|
||||
message: `HTTP ${response.status}${response.statusText ? `: ${response.statusText}` : ""}`,
|
||||
findings: [],
|
||||
};
|
||||
return data.detail ?? `HTTP ${response.status}: ${response.statusText}`;
|
||||
}
|
||||
|
||||
export async function loadSkills() {
|
||||
const skills = await fetch(`${getBackendBaseURL()}/api/skills`);
|
||||
if (!skills.ok) {
|
||||
throw new SkillRequestError(skills.status, await readErrorDetail(skills));
|
||||
const detail = await readErrorDetail(skills);
|
||||
throw new SkillRequestError(skills.status, detail.message, detail);
|
||||
}
|
||||
const json = await skills.json();
|
||||
return json.skills as Skill[];
|
||||
@ -47,10 +145,8 @@ export async function enableSkill(skillName: string, enabled: boolean) {
|
||||
},
|
||||
);
|
||||
if (!response.ok) {
|
||||
throw new SkillRequestError(
|
||||
response.status,
|
||||
await readErrorDetail(response),
|
||||
);
|
||||
const detail = await readErrorDetail(response);
|
||||
throw new SkillRequestError(response.status, detail.message, detail);
|
||||
}
|
||||
return response.json();
|
||||
}
|
||||
@ -78,17 +174,50 @@ export async function installSkill(
|
||||
});
|
||||
|
||||
if (!response.ok) {
|
||||
const message = await readErrorDetail(response);
|
||||
const detail = await readErrorDetail(response);
|
||||
// Surface authorization failures so callers can show an admin-only hint
|
||||
// instead of a generic failure.
|
||||
if (response.status === 403) {
|
||||
throw new SkillRequestError(response.status, message);
|
||||
throw new SkillRequestError(response.status, detail.message, detail);
|
||||
}
|
||||
// Other HTTP errors keep the existing soft-failure contract.
|
||||
return {
|
||||
success: false,
|
||||
skill_name: "",
|
||||
message,
|
||||
message: detail.message,
|
||||
};
|
||||
}
|
||||
|
||||
return response.json();
|
||||
}
|
||||
|
||||
export async function uploadSkillArchive(
|
||||
archive: File,
|
||||
): Promise<InstallSkillResponse> {
|
||||
const formData = new FormData();
|
||||
formData.append("archive", archive);
|
||||
|
||||
const response = await fetch(
|
||||
`${getBackendBaseURL()}/api/skills/install/upload`,
|
||||
{
|
||||
method: "POST",
|
||||
body: formData,
|
||||
},
|
||||
);
|
||||
|
||||
if (!response.ok) {
|
||||
const detail = await readErrorDetail(response);
|
||||
if (
|
||||
response.status === 403 ||
|
||||
response.status === 413 ||
|
||||
detail.findings.length > 0
|
||||
) {
|
||||
throw new SkillRequestError(response.status, detail.message, detail);
|
||||
}
|
||||
return {
|
||||
success: false,
|
||||
skill_name: "",
|
||||
message: detail.message,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@ -1,6 +1,6 @@
|
||||
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
|
||||
|
||||
import { enableSkill, SkillRequestError } from "./api";
|
||||
import { enableSkill, SkillRequestError, uploadSkillArchive } from "./api";
|
||||
|
||||
import { loadSkills } from ".";
|
||||
|
||||
@ -30,3 +30,15 @@ export function useEnableSkill() {
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
export function useUploadSkillArchive() {
|
||||
const queryClient = useQueryClient();
|
||||
return useMutation({
|
||||
mutationFn: uploadSkillArchive,
|
||||
onSuccess: (result) => {
|
||||
if (result.success) {
|
||||
void queryClient.invalidateQueries({ queryKey: ["skills"] });
|
||||
}
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
154
frontend/tests/unit/core/skills/api.test.ts
Normal file
154
frontend/tests/unit/core/skills/api.test.ts
Normal file
@ -0,0 +1,154 @@
|
||||
import { beforeEach, describe, expect, rs, test } from "@rstest/core";
|
||||
|
||||
rs.mock("@/core/api/fetcher", () => ({
|
||||
fetch: rs.fn(),
|
||||
}));
|
||||
|
||||
rs.mock("@/core/config", () => ({
|
||||
getBackendBaseURL: () => "/backend",
|
||||
}));
|
||||
|
||||
import { fetch as fetcher } from "@/core/api/fetcher";
|
||||
import {
|
||||
formatSkillSecurityFindings,
|
||||
SkillRequestError,
|
||||
uploadSkillArchive,
|
||||
} from "@/core/skills/api";
|
||||
|
||||
const mockedFetch = rs.mocked(fetcher);
|
||||
|
||||
function jsonResponse(status: number, body: unknown): Response {
|
||||
return new Response(JSON.stringify(body), {
|
||||
status,
|
||||
statusText: status >= 400 ? "Error" : "OK",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
});
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
mockedFetch.mockReset();
|
||||
});
|
||||
|
||||
describe("skills api", () => {
|
||||
test("uploads a local .skill archive as multipart form data", async () => {
|
||||
mockedFetch.mockResolvedValueOnce(
|
||||
jsonResponse(200, {
|
||||
success: true,
|
||||
skill_name: "demo",
|
||||
message: "Installed demo",
|
||||
}),
|
||||
);
|
||||
const archive = new File(["archive"], "demo.skill", {
|
||||
type: "application/octet-stream",
|
||||
});
|
||||
|
||||
await expect(uploadSkillArchive(archive)).resolves.toEqual({
|
||||
success: true,
|
||||
skill_name: "demo",
|
||||
message: "Installed demo",
|
||||
});
|
||||
|
||||
expect(mockedFetch).toHaveBeenCalledTimes(1);
|
||||
const [url, init] = mockedFetch.mock.calls[0]!;
|
||||
expect(url).toBe("/backend/api/skills/install/upload");
|
||||
expect(init?.method).toBe("POST");
|
||||
expect(init?.headers).toBeUndefined();
|
||||
expect(init?.body).toBeInstanceOf(FormData);
|
||||
expect((init?.body as FormData).get("archive")).toBe(archive);
|
||||
});
|
||||
|
||||
test("preserves the admin-required error contract", async () => {
|
||||
mockedFetch.mockResolvedValueOnce(
|
||||
jsonResponse(403, { detail: "Admin privileges required" }),
|
||||
);
|
||||
|
||||
await expect(
|
||||
uploadSkillArchive(new File(["archive"], "demo.skill")),
|
||||
).rejects.toEqual(
|
||||
expect.objectContaining({
|
||||
name: "SkillRequestError",
|
||||
status: 403,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
test("preserves structured security findings for the upload UI", async () => {
|
||||
mockedFetch.mockResolvedValueOnce(
|
||||
jsonResponse(400, {
|
||||
detail: {
|
||||
message: "Static security scan blocked skill 'demo'",
|
||||
skill_name: "demo",
|
||||
findings: [
|
||||
{
|
||||
rule_id: "python-shell-exec",
|
||||
severity: "HIGH",
|
||||
file: "scripts/run.py",
|
||||
line: 7,
|
||||
message: "Python invokes a shell command.",
|
||||
remediation: "Remove the shell call.",
|
||||
},
|
||||
],
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
const error = await uploadSkillArchive(
|
||||
new File(["archive"], "demo.skill"),
|
||||
).catch((caught: unknown) => caught);
|
||||
|
||||
expect(error).toEqual(
|
||||
expect.objectContaining({
|
||||
name: "SkillRequestError",
|
||||
status: 400,
|
||||
skillName: "demo",
|
||||
findings: [
|
||||
expect.objectContaining({
|
||||
rule_id: "python-shell-exec",
|
||||
file: "scripts/run.py",
|
||||
line: 7,
|
||||
}),
|
||||
],
|
||||
}),
|
||||
);
|
||||
if (!(error instanceof SkillRequestError)) {
|
||||
throw new Error("expected a SkillRequestError");
|
||||
}
|
||||
expect(formatSkillSecurityFindings(error.findings)).toBe(
|
||||
"HIGH python-shell-exec · scripts/run.py:7: Python invokes a shell command. Remove the shell call.",
|
||||
);
|
||||
});
|
||||
|
||||
test("turns an unstructured proxy 413 into a typed request error", async () => {
|
||||
mockedFetch.mockResolvedValueOnce(
|
||||
new Response("", { status: 413, statusText: "" }),
|
||||
);
|
||||
|
||||
await expect(
|
||||
uploadSkillArchive(new File(["archive"], "demo.skill")),
|
||||
).rejects.toEqual(
|
||||
expect.objectContaining({
|
||||
name: "SkillRequestError",
|
||||
status: 413,
|
||||
message: "HTTP 413",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
test("limits formatted findings and reports how many remain", () => {
|
||||
const findings = Array.from({ length: 5 }, (_, index) => ({
|
||||
rule_id: `rule-${index + 1}`,
|
||||
severity: "HIGH",
|
||||
file: `scripts/run-${index + 1}.py`,
|
||||
line: index + 1,
|
||||
message: `Finding ${index + 1}.`,
|
||||
remediation: null,
|
||||
}));
|
||||
|
||||
expect(formatSkillSecurityFindings(findings).split("\n")).toEqual([
|
||||
"HIGH rule-1 · scripts/run-1.py:1: Finding 1.",
|
||||
"HIGH rule-2 · scripts/run-2.py:2: Finding 2.",
|
||||
"HIGH rule-3 · scripts/run-3.py:3: Finding 3.",
|
||||
"... and 2 more",
|
||||
]);
|
||||
});
|
||||
});
|
||||
26
scripts/check_chart_skill_upload_size.sh
Executable file
26
scripts/check_chart_skill_upload_size.sh
Executable file
@ -0,0 +1,26 @@
|
||||
#!/usr/bin/env bash
|
||||
# Assert the rendered outer Ingress preserves the Gateway's .skill upload policy.
|
||||
set -euo pipefail
|
||||
|
||||
repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
|
||||
rendered="$(mktemp)"
|
||||
trap 'rm -f "$rendered"' EXIT
|
||||
|
||||
helm template deer-flow "$repo_root/deploy/helm/deer-flow" --include-crds >"$rendered"
|
||||
|
||||
if ! grep -Eq 'nginx\.ingress\.kubernetes\.io/proxy-body-size: "?101m"?[[:space:]]*$' "$rendered"; then
|
||||
echo "Rendered Ingress must allow 101m for 100 MiB .skill uploads plus multipart framing." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
if ! grep -Eq 'nginx\.ingress\.kubernetes\.io/proxy-request-buffering: "?off"?[[:space:]]*$' "$rendered"; then
|
||||
echo "Rendered Ingress must stream .skill uploads without request buffering." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
if ! grep -Eq 'nginx\.ingress\.kubernetes\.io/proxy-read-timeout: "?600"?[[:space:]]*$' "$rendered"; then
|
||||
echo "Rendered Ingress must allow 600 seconds for .skill upload validation." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
echo "Chart skill-upload ingress policy check passed."
|
||||
Loading…
x
Reference in New Issue
Block a user