From 99fd1ff82ed056c91128f15c0b00d2ab5f360a53 Mon Sep 17 00:00:00 2001 From: ciregenz Date: Wed, 5 Aug 2026 01:20:17 -0700 Subject: [PATCH] [eric] skills: /upload endpoint takes a bare SKILL .md or a .zip/.skill archive, shallowest SKILL.md wins --- backend/apps/skills/models.py | 5 + backend/apps/skills/skills.py | 56 ++++- backend/tests/test_skill_upload.py | 91 +++++++ .../pages/Skills/CommunitySkillsDialog.tsx | 222 ------------------ 4 files changed, 151 insertions(+), 223 deletions(-) create mode 100644 backend/tests/test_skill_upload.py delete mode 100644 frontend/src/app/pages/Skills/CommunitySkillsDialog.tsx diff --git a/backend/apps/skills/models.py b/backend/apps/skills/models.py index e27fe05e..4b7bf08a 100644 --- a/backend/apps/skills/models.py +++ b/backend/apps/skills/models.py @@ -39,6 +39,11 @@ class SkillLoadRequest(BaseModel): id: str +class SkillUpload(BaseModel): + filename: str + content_b64: str + + class SkillWorkspaceSeedRequest(BaseModel): workspace_id: str skill_content: Optional[str] = None diff --git a/backend/apps/skills/skills.py b/backend/apps/skills/skills.py index 1f0d6ff6..1e629604 100644 --- a/backend/apps/skills/skills.py +++ b/backend/apps/skills/skills.py @@ -1,3 +1,6 @@ +import base64 +import binascii +import io import os import hashlib import json @@ -6,10 +9,11 @@ import re import tempfile import threading import time +import zipfile from contextlib import asynccontextmanager from fastapi import HTTPException from backend.config.Apps import SubApp -from backend.apps.skills.models import Skill, SkillCreate, SkillLoadRequest, SkillUpdate, SkillWorkspaceSeedRequest +from backend.apps.skills.models import Skill, SkillCreate, SkillLoadRequest, SkillUpdate, SkillUpload, SkillWorkspaceSeedRequest logger = logging.getLogger(__name__) @@ -473,6 +477,56 @@ def write_folder_skill(skill_id: str, files: dict[str, str], meta: dict) -> Skil return p_build_skill(slug, content, md_path, kind, index) +@skills.router.post("/upload") +async def upload_skill(body: SkillUpload): + """The Directory's Upload skill drop zone: a bare SKILL .md, or a .zip/.skill archive + whose shallowest SKILL.md marks the skill root; sibling files ride along as folder extras.""" + name_l = body.filename.lower() + try: + raw = base64.b64decode(body.content_b64) + except (binascii.Error, ValueError): + raise HTTPException(status_code=400, detail="upload was not valid base64") + + if name_l.endswith(".md"): + text = raw.decode("utf-8", errors="replace") + meta = p_parse_skill_frontmatter(text) + if not meta.get("name") or not meta.get("description"): + raise HTTPException(status_code=400, detail=".md file must contain skill name and description formatted in YAML") + skill = write_folder_skill(unique_skill_slug(meta["name"]), {"SKILL.md": text}, meta) + return {"ok": True, "skill": skill.model_dump()} + + if name_l.endswith(".zip") or name_l.endswith(".skill"): + try: + zf = zipfile.ZipFile(io.BytesIO(raw)) + except zipfile.BadZipFile: + raise HTTPException(status_code=400, detail="file is not a valid zip archive") + entries = [n for n in zf.namelist() if not n.endswith("/")] + md_entries = [n for n in entries if n.split("/")[-1] == "SKILL.md"] + if not md_entries: + raise HTTPException(status_code=400, detail=".zip or .skill file must include a SKILL.md file") + md_entry = min(md_entries, key=lambda n: n.count("/")) + root = md_entry[: -len("SKILL.md")] + files: dict[str, str] = {} + for n in entries: + if not n.startswith(root): + continue + rel = n[len(root):] + if not rel: + continue + try: + files[rel] = zf.read(n).decode("utf-8") + except UnicodeDecodeError: + # Binary assets are skipped; the skill contract is text (SKILL.md + scripts). + logger.warning("skill upload: skipped binary entry %r", n) + meta = p_parse_skill_frontmatter(files.get("SKILL.md", "")) + if not meta.get("name"): + meta["name"] = re.sub(r"\.(zip|skill)$", "", body.filename, flags=re.IGNORECASE) + skill = write_folder_skill(unique_skill_slug(meta["name"]), files, meta) + return {"ok": True, "skill": skill.model_dump()} + + raise HTTPException(status_code=400, detail="unsupported file type: upload a .md, .zip, or .skill file") + + @skills.router.post("/create") async def create_skill(body: SkillCreate): # All user skills are folders now (/SKILL.md); flat files stay readable but are no longer written, so a skill's on-disk shape no longer depends on how it was created vs imported. diff --git a/backend/tests/test_skill_upload.py b/backend/tests/test_skill_upload.py new file mode 100644 index 00000000..4979da5f --- /dev/null +++ b/backend/tests/test_skill_upload.py @@ -0,0 +1,91 @@ +"""The Directory's Upload skill endpoint: a bare SKILL .md needs YAML name+description, +a .zip/.skill archive needs a SKILL.md (shallowest wins, siblings ride along), and +anything else is refused with a readable reason.""" + +from __future__ import annotations + +import base64 +import io +import zipfile + +import pytest +from fastapi import HTTPException + +import backend.apps.skills.skills as skills_mod +from backend.apps.skills.models import SkillUpload + + +@pytest.fixture +def isolated_skills(tmp_path, monkeypatch): + d = tmp_path / "skills" + d.mkdir() + monkeypatch.setattr(skills_mod, "SKILLS_DIR", str(d)) + monkeypatch.setattr(skills_mod, "INDEX_PATH", str(d / ".skills_index.json")) + return d + + +def b64(data: bytes) -> str: + return base64.b64encode(data).decode() + + +def make_zip(entries: dict[str, str]) -> bytes: + buf = io.BytesIO() + with zipfile.ZipFile(buf, "w") as zf: + for name, content in entries.items(): + zf.writestr(name, content) + return buf.getvalue() + + +@pytest.mark.asyncio +async def test_md_upload_creates_skill(isolated_skills): + md = "---\nname: Test Upload\ndescription: A test\n---\n\n# Test Upload\n" + res = await skills_mod.upload_skill(SkillUpload(filename="test.md", content_b64=b64(md.encode()))) + assert res["ok"] is True + assert res["skill"]["name"] == "Test Upload" + assert (isolated_skills / "test-upload" / "SKILL.md").is_file() + + +@pytest.mark.asyncio +async def test_md_without_frontmatter_rejected(isolated_skills): + with pytest.raises(HTTPException) as e: + await skills_mod.upload_skill(SkillUpload(filename="x.md", content_b64=b64(b"no yaml"))) + assert e.value.status_code == 400 + assert ".md file must contain skill name and description formatted in YAML" in e.value.detail + + +@pytest.mark.asyncio +async def test_zip_with_nested_skill_md(isolated_skills): + raw = make_zip({ + "my-skill/SKILL.md": "---\nname: Zipped\ndescription: d\n---\nbody", + "my-skill/scripts/run.py": "print('hi')", + "unrelated/readme.txt": "not part of the skill", + }) + res = await skills_mod.upload_skill(SkillUpload(filename="my-skill.zip", content_b64=b64(raw))) + assert res["ok"] is True + base = isolated_skills / "zipped" + assert (base / "SKILL.md").is_file() + assert (base / "scripts" / "run.py").is_file() + assert not (base / "readme.txt").exists() + + +@pytest.mark.asyncio +async def test_zip_without_skill_md_rejected(isolated_skills): + raw = make_zip({"folder/notes.md": "just notes"}) + with pytest.raises(HTTPException) as e: + await skills_mod.upload_skill(SkillUpload(filename="x.zip", content_b64=b64(raw))) + assert e.value.status_code == 400 + assert ".zip or .skill file must include a SKILL.md file" in e.value.detail + + +@pytest.mark.asyncio +async def test_unsupported_extension_rejected(isolated_skills): + with pytest.raises(HTTPException) as e: + await skills_mod.upload_skill(SkillUpload(filename="x.tar.gz", content_b64=b64(b"whatever"))) + assert e.value.status_code == 400 + + +@pytest.mark.asyncio +async def test_bad_base64_rejected(isolated_skills): + with pytest.raises(HTTPException) as e: + await skills_mod.upload_skill(SkillUpload(filename="x.md", content_b64="!!!not-base64!!!")) + assert e.value.status_code == 400 diff --git a/frontend/src/app/pages/Skills/CommunitySkillsDialog.tsx b/frontend/src/app/pages/Skills/CommunitySkillsDialog.tsx deleted file mode 100644 index cb4398a5..00000000 --- a/frontend/src/app/pages/Skills/CommunitySkillsDialog.tsx +++ /dev/null @@ -1,222 +0,0 @@ -import React, { useEffect, useRef, useState, useCallback } from 'react'; -import Dialog from '@mui/material/Dialog'; -import DialogTitle from '@mui/material/DialogTitle'; -import DialogContent from '@mui/material/DialogContent'; -import DialogActions from '@mui/material/DialogActions'; -import Box from '@mui/material/Box'; -import Typography from '@mui/material/Typography'; -import TextField from '@mui/material/TextField'; -import Button from '@mui/material/Button'; -import Chip from '@mui/material/Chip'; -import CircularProgress from '@mui/material/CircularProgress'; -import Alert from '@mui/material/Alert'; -import InputAdornment from '@mui/material/InputAdornment'; -import SearchIcon from '@mui/icons-material/Search'; -import { EmptyState } from '@/app/components/feedback/Loading'; -import OpenInNewIcon from '@mui/icons-material/OpenInNew'; -import WarningAmberIcon from '@mui/icons-material/WarningAmber'; -import { useClaudeTokens } from '@/shared/styles/ThemeContext'; -import { - searchCommunitySkills, - installCommunitySkill, - CommunitySkill, - InstallDisclosure, -} from '@/shared/state/skillRegistrySlice'; - -interface Props { - open: boolean; - onClose: () => void; - onInstalled: (name: string) => void; -} - -// The skills.sh wild registry is unvetted community code (skills can ship scripts). So this dialog never installs blind: picking a skill fetches a disclosure (files + scripts) the user confirms before anything lands on disk. -const CommunitySkillsDialog: React.FC = ({ open, onClose, onInstalled }) => { - const c = useClaudeTokens(); - const [query, setQuery] = useState(''); - const [results, setResults] = useState([]); - const [loading, setLoading] = useState(false); - const [selected, setSelected] = useState(null); - const [disclosure, setDisclosure] = useState(null); - const [busy, setBusy] = useState(false); - const [error, setError] = useState(null); - const debounceRef = useRef | null>(null); - // Monotonic request tokens: a slow response from an earlier search/preview must not overwrite the state a newer one already set (out-of-order network). - const searchSeq = useRef(0); - const previewSeq = useRef(0); - - const runSearch = useCallback(async (q: string) => { - const seq = ++searchSeq.current; - setLoading(true); - setError(null); - try { - const res = await searchCommunitySkills(q); - if (seq !== searchSeq.current) return; - setResults(res); - } catch (e) { - if (seq !== searchSeq.current) return; - setError(e instanceof Error ? e.message : 'Search failed'); - setResults([]); - } finally { - if (seq === searchSeq.current) setLoading(false); - } - }, []); - - useEffect(() => { - if (!open) return; - if (debounceRef.current) clearTimeout(debounceRef.current); - debounceRef.current = setTimeout(() => runSearch(query.trim()), 300); - return () => { if (debounceRef.current) clearTimeout(debounceRef.current); }; - }, [query, open, runSearch]); - - useEffect(() => { - if (!open) { - setQuery(''); setResults([]); setSelected(null); setDisclosure(null); setError(null); - } - }, [open]); - - const preview = async (skill: CommunitySkill) => { - const seq = ++previewSeq.current; - setSelected(skill); - setDisclosure(null); - setBusy(true); - setError(null); - try { - const res = await installCommunitySkill(skill.source, skill.skillId, false); - if (seq !== previewSeq.current) return; - setDisclosure(res.disclosure); - } catch (e) { - if (seq !== previewSeq.current) return; - setError(e instanceof Error ? e.message : 'Could not load skill'); - setSelected(null); - } finally { - if (seq === previewSeq.current) setBusy(false); - } - }; - - const confirmInstall = async () => { - if (!selected) return; - setBusy(true); - setError(null); - try { - await installCommunitySkill(selected.source, selected.skillId, true); - onInstalled(disclosure?.name || selected.name); - setSelected(null); - setDisclosure(null); - } catch (e) { - setError(e instanceof Error ? e.message : 'Install failed'); - } finally { - setBusy(false); - } - }; - - return ( - - - Browse community skills - - From the skills.sh registry. Community-published and unvetted; you'll see exactly what installs before it lands. - - - - {error && {error}} - - {!selected && ( - <> - setQuery(e.target.value)} - size="small" - InputProps={{ startAdornment: () }} - /> - {loading && } - {!loading && results.length === 0 && ( - - )} - {!loading && results.map((s) => ( - preview(s)} - sx={{ - px: 1.5, py: 1, borderRadius: `${c.radius.sm}px`, cursor: 'pointer', - border: `1px solid ${c.border.subtle}`, - '&:hover': { borderColor: c.accent.primary, bgcolor: `${c.accent.primary}08` }, - }}> - - {s.name} - - - {s.source} - - ))} - - )} - - {selected && ( - - - {busy && !disclosure && } - {disclosure && ( - <> - {disclosure.name} - {disclosure.description && ( - {disclosure.description} - )} - - {selected.source} - - - {/* The real risk for an agent platform: SKILL.md is injected as instructions the - agent follows, with its full tool surface. Say that plainly, not just "scripts". */} - } sx={{ fontSize: '0.75rem', py: 0 }}> - This is an unvetted community skill. Its SKILL.md becomes instructions your agent will follow, and it can use your agent's tools (files, browser, settings). Only install from a source you trust, read it below first. - - - {disclosure.secret_findings.length > 0 && ( - } sx={{ fontSize: '0.75rem', py: 0 }}> - {disclosure.secret_findings.length} file{disclosure.secret_findings.length === 1 ? '' : 's'} contain secret-shaped text ({disclosure.secret_findings.slice(0, 3).join(', ')}{disclosure.secret_findings.length > 3 ? '…' : ''}). A trustworthy skill shouldn't ship credentials; treat this as a red flag. - - )} - - {disclosure.has_scripts && ( - } sx={{ fontSize: '0.75rem', py: 0 }}> - Includes {disclosure.scripts.length} script file{disclosure.scripts.length === 1 ? '' : 's'} that can run code when an agent uses this skill. Installing only writes the files; nothing runs until an agent does, and that still goes through normal command approval. - - )} - - - {disclosure.files.length} file{disclosure.files.length === 1 ? '' : 's'} will be installed: - - - {disclosure.files.map((f) => ( - - {disclosure.scripts.includes(f) ? '⚙ ' : ''}{f} - - ))} - - - )} - - )} - - - - {selected && disclosure && ( - - )} - - - ); -}; - -export default CommunitySkillsDialog;