diff --git a/backend/apps/swarm/closure.py b/backend/apps/swarm/closure.py index 6010ef85..b841ca00 100644 --- a/backend/apps/swarm/closure.py +++ b/backend/apps/swarm/closure.py @@ -197,6 +197,16 @@ def stage_upload(raw: bytes, filename: str) -> tuple[str, Manifest, list[str]]: sandbox = unpack(raw) try: raw_manifest = read_manifest(sandbox) + except BundleError: + shutil.rmtree(sandbox, ignore_errors=True) + raise + # "manifest.json" is a name a dozen other tools use; a skill zip that happens to carry + # one is not a broken bundle, it is not a bundle at all. Decide on the CONTENT, so a + # tampered bundle of ours still fails loudly on the strict path below. + if not looks_like_our_manifest(raw_manifest): + shutil.rmtree(sandbox, ignore_errors=True) + return stage_skill_from_zip(raw, filename, warnings) + try: verify_checksum(sandbox, raw_manifest) manifest = Manifest(**raw_manifest) validate_manifest(manifest) @@ -217,6 +227,14 @@ def stage_upload(raw: bytes, filename: str) -> tuple[str, Manifest, list[str]]: return p_stage_skill_from_markdown(raw, filename, warnings) +def looks_like_our_manifest(raw_manifest: object) -> bool: + """Ours, by the three keys only a .swarm manifest has. A tampered or truncated bundle of ours + still passes this and goes on to fail its checksum, which is the half that must not get lost.""" + if not isinstance(raw_manifest, dict): + return False + return all(key in raw_manifest for key in ("bundle_id", "root", "entities")) + + def p_name_from_filename(filename: str) -> str: base = os.path.splitext(os.path.basename(filename or "skill"))[0] return base.replace("-", " ").replace("_", " ").strip().title() or "Imported Skill" diff --git a/backend/tests/test_import_decides_on_content.py b/backend/tests/test_import_decides_on_content.py new file mode 100644 index 00000000..e73cf1f9 --- /dev/null +++ b/backend/tests/test_import_decides_on_content.py @@ -0,0 +1,62 @@ +"""A zip is judged by what its manifest CONTAINS, not by the fact that one is named manifest.json. + +Found live 2026-09-02 on a package published to the marketplace: a plain skill zip carrying its own +unrelated `manifest.json` (name/kind/version) was rejected as "bundle manifest is invalid", because +staging keyed on the filename alone. The half that must not get lost while fixing that is the +strict path: anything shaped like OUR manifest still has to fail loudly when it has been tampered +with, rather than being quietly reinterpreted as a skill. +""" + +import io +import json +import shutil +import zipfile + +import pytest + +from backend.apps.swarm import closure +from backend.apps.swarm.models import EntityType +from backend.apps.swarm.ziputil import BundleError + +FOREIGN_MANIFEST = {"name": "hello-world", "kind": "skill", "version": "1.0.0", "description": "hi"} +SKILL_MD = "---\nname: hello-world\ndescription: Says hello\n---\n\nSay hello.\n" + + +def p_zip(members: dict) -> bytes: + buf = io.BytesIO() + with zipfile.ZipFile(buf, "w", zipfile.ZIP_DEFLATED) as z: + for name, body in members.items(): + z.writestr(name, body) + return buf.getvalue() + + +def test_a_skill_zip_with_someone_elses_manifest_installs_as_a_skill(): + raw = p_zip({"manifest.json": json.dumps(FOREIGN_MANIFEST), "SKILL.md": SKILL_MD}) + sandbox, manifest, warnings = closure.stage_upload(raw, "hello-world.swarm") + try: + assert manifest.root.type == EntityType.skill + assert closure.summarize(manifest).root.type == EntityType.skill + finally: + shutil.rmtree(sandbox, ignore_errors=True) + + +def test_a_manifest_shaped_like_ours_still_fails_loudly_when_it_is_broken(): + """The negative control for the fallthrough: a damaged bundle of ours must never be salvaged + into a skill install, because that would turn a tamper check into a silent downgrade.""" + ours = {"bundle_id": "abc", "root": {"type": "app", "bundle_id": "abc", "name": "X", "path": "entities/abc"}, + "entities": [], "checksum": "0" * 64, "format_version": 1} + raw = p_zip({"manifest.json": json.dumps(ours), "SKILL.md": SKILL_MD}) + with pytest.raises(BundleError): + closure.stage_upload(raw, "tampered.swarm") + + +def test_the_discriminator_reads_the_three_keys_only_our_manifest_has(): + assert closure.looks_like_our_manifest({"bundle_id": "a", "root": {}, "entities": []}) is True + assert closure.looks_like_our_manifest(FOREIGN_MANIFEST) is False + assert closure.looks_like_our_manifest(["not", "a", "dict"]) is False + + +def test_a_zip_that_is_neither_says_so_rather_than_installing_something(): + raw = p_zip({"manifest.json": json.dumps(FOREIGN_MANIFEST), "README.txt": "nothing to install"}) + with pytest.raises(BundleError): + closure.stage_upload(raw, "empty.swarm") diff --git a/frontend/src/app/pages/Directory/DirectorySkillsTab.tsx b/frontend/src/app/pages/Directory/DirectorySkillsTab.tsx deleted file mode 100644 index 6cdb37d3..00000000 --- a/frontend/src/app/pages/Directory/DirectorySkillsTab.tsx +++ /dev/null @@ -1,299 +0,0 @@ -import React, { useEffect, useMemo, useRef, useState } from 'react'; -import Box from '@mui/material/Box'; -import Typography from '@mui/material/Typography'; -import IconButton from '@mui/material/IconButton'; -import CircularProgress from '@mui/material/CircularProgress'; -import Snackbar from '@mui/material/Snackbar'; -import Alert from '@mui/material/Alert'; -import AddIcon from '@mui/icons-material/Add'; -import SettingsOutlinedIcon from '@mui/icons-material/SettingsOutlined'; -import FileDownloadOutlinedIcon from '@mui/icons-material/FileDownloadOutlined'; -import { useClaudeTokens } from '@/shared/styles/ThemeContext'; -import { useAppDispatch, useAppSelector } from '@/shared/hooks'; -import { fetchSkills } from '@/shared/state/skillsSlice'; -import { - fetchAllRegistrySkills, - installCuratedSkill, - INSTALL_COMMAND_RE, - parseInstallCommand, - searchCommunitySkills, - CommunitySkill, - RegistrySkill, -} from '@/shared/state/skillRegistrySlice'; -import DirectoryFilterBar from './DirectoryFilterBar'; -import CommunityInstallConfirm from './dialogs/CommunityInstallConfirm'; - -interface Props { - onOpenInstalled?: (skillId: string) => void; -} - -interface SkillCardModel { - key: string; - slug: string; - publisher: string; - description: string; - installs: number | null; - isCommunity: boolean; - curated?: RegistrySkill; - community?: CommunitySkill; -} - -export function formatInstallCount(n: number): string { - if (n >= 1_000_000) return `${(n / 1_000_000).toFixed(1).replace(/\.0$/, '')}M`; - if (n >= 1_000) return `${(n / 1_000).toFixed(1).replace(/\.0$/, '')}K`; - return String(n); -} - -const DirectorySkillsTab: React.FC = ({ onOpenInstalled }) => { - const c = useClaudeTokens(); - const dispatch = useAppDispatch(); - const { skills: curated, loading: curatedLoading } = useAppSelector((s) => s.skillRegistry); - const localSkills = useAppSelector((s) => s.skills.items); - - const [query, setQuery] = useState(''); - // claude.ai grammar: Filter by is checkable sections. Community starts unchecked so the landing view is the Anthropic set, same as claude.ai's. - const [filterSelected, setFilterSelected] = useState(['installed', 'not-installed', 'anthropic']); - const [sort, setSort] = useState('popular'); - const toggleFilter = (value: string) => setFilterSelected((p) => (p.includes(value) ? p.filter((v) => v !== value) : [...p, value])); - const [community, setCommunity] = useState([]); - const [communityLoading, setCommunityLoading] = useState(false); - const [installingKey, setInstallingKey] = useState(null); - const [confirmTarget, setConfirmTarget] = useState(null); - const [snackbar, setSnackbar] = useState<{ open: boolean; message: string; severity: 'success' | 'error' }>({ open: false, message: '', severity: 'success' }); - const debounceRef = useRef | null>(null); - const searchSeq = useRef(0); - - useEffect(() => { - if (curated.length === 0) dispatch(fetchAllRegistrySkills()); - dispatch(fetchSkills()); - }, [dispatch, curated.length]); - - // skills.sh is the npx-installable community registry; its results carry install counts. - useEffect(() => { - if (debounceRef.current) clearTimeout(debounceRef.current); - debounceRef.current = setTimeout(async () => { - const seq = ++searchSeq.current; - setCommunityLoading(true); - try { - // A pasted README install command ("npx skills add pdf-filler", a skills.sh URL) resolves to - // its skill id server-side first, so the box accepts the grammar people actually copy (ENG-217). - let effective = query.trim(); - if (INSTALL_COMMAND_RE.test(effective)) { - const skillId = await parseInstallCommand(effective).catch(() => null); - if (skillId) effective = skillId; - } - const res = await searchCommunitySkills(effective); - if (seq === searchSeq.current) setCommunity(res); - } catch { - if (seq === searchSeq.current) setCommunity([]); - } finally { - if (seq === searchSeq.current) setCommunityLoading(false); - } - }, 300); - return () => { if (debounceRef.current) clearTimeout(debounceRef.current); }; - }, [query]); - - const installedNames = useMemo( - () => new Set(Object.values(localSkills).map((s) => s.name.trim().toLowerCase())), - [localSkills], - ); - const localIdByName = useMemo(() => { - const m: Record = {}; - for (const s of Object.values(localSkills)) m[s.name.trim().toLowerCase()] = s.id; - return m; - }, [localSkills]); - - const cards = useMemo((): SkillCardModel[] => { - // A pasted install command already resolved server-side into the community results; filtering - // those hits against the raw paste string would hide the exact skill the user asked for. - const q = INSTALL_COMMAND_RE.test(query.trim()) ? '' : query.trim().toLowerCase(); - // Join curated skills to their skills.sh twin (source anthropics/skills) so Anthropic cards get real install counts. - const communityByName = new Map(community.map((cs) => [cs.name.trim().toLowerCase(), cs])); - const out: SkillCardModel[] = []; - if (filterSelected.includes('anthropic')) { - for (const sk of curated) { - if (q && !sk.name.toLowerCase().includes(q) && !sk.description.toLowerCase().includes(q)) continue; - const twin = communityByName.get(sk.name.trim().toLowerCase()); - out.push({ - key: `curated:${sk.folder}`, - slug: sk.name.toLowerCase().replace(/\s+/g, '-'), - publisher: 'Anthropic', - description: sk.description, - installs: twin && /anthropic/i.test(twin.source) ? twin.installs : null, - isCommunity: false, - curated: sk, - }); - } - } - if (filterSelected.includes('community')) { - const curatedNames = new Set(curated.map((sk) => sk.name.trim().toLowerCase())); - for (const cs of community) { - // Anthropic-sourced twins already render as curated cards; listing them twice reads as duplicates. - if (/anthropic/i.test(cs.source) && curatedNames.has(cs.name.trim().toLowerCase())) continue; - if (q && !cs.name.toLowerCase().includes(q) && !cs.source.toLowerCase().includes(q)) continue; - out.push({ - key: `community:${cs.source}/${cs.skillId}`, - slug: cs.skillId.toLowerCase().replace(/\s+/g, '-'), - publisher: cs.source, - // skills.sh sometimes echoes the install count as the description; the meta line already carries it. - description: /^[\d,.]+ installs?$/.test(cs.description.trim()) ? '' : cs.description, - installs: cs.installs, - isCommunity: true, - community: cs, - }); - } - } - const statusOk = (installed: boolean): boolean => (installed ? filterSelected.includes('installed') : filterSelected.includes('not-installed')); - const withStatus = out.filter((card) => statusOk(installedNames.has((card.curated?.name ?? card.community?.name ?? '').trim().toLowerCase()))); - if (sort === 'name') withStatus.sort((a, b) => a.slug.localeCompare(b.slug)); - else { - // claude.ai's landing order: installed first, then popularity descending. - const rank = (card: SkillCardModel): number => (installedNames.has((card.curated?.name ?? card.community?.name ?? '').trim().toLowerCase()) ? 1 : 0); - withStatus.sort((a, b) => rank(b) - rank(a) || (b.installs ?? -1) - (a.installs ?? -1)); - } - return withStatus; - }, [curated, community, query, filterSelected, sort, installedNames]); - - const handleInstallCurated = async (card: SkillCardModel) => { - if (!card.curated) return; - setInstallingKey(card.key); - try { - await dispatch(installCuratedSkill(card.curated.folder)).unwrap(); - await dispatch(fetchSkills()); - setSnackbar({ open: true, message: `Installed "${card.curated.name}"`, severity: 'success' }); - } catch (e) { - const msg = (e as { message?: string })?.message || 'unknown error'; - setSnackbar({ open: true, message: `Install failed: ${msg}`, severity: 'error' }); - } finally { - setInstallingKey(null); - } - }; - - const isInstalled = (card: SkillCardModel): boolean => { - const name = (card.curated?.name ?? card.community?.name ?? '').trim().toLowerCase(); - return installedNames.has(name); - }; - - return ( - - - - - {(curatedLoading && curated.length === 0) || (communityLoading && cards.length === 0) ? ( - - - - ) : cards.length === 0 ? ( - - No skills match your search. - - ) : cards.map((card) => { - const installed = isInstalled(card); - return ( - - - - /{card.slug} - - {installingKey === card.key ? ( - - ) : installed ? ( - { - const id = localIdByName[(card.curated?.name ?? card.community?.name ?? '').trim().toLowerCase()]; - if (id && onOpenInstalled) onOpenInstalled(id); - }} - sx={{ color: c.text.tertiary, '&:hover': { color: c.text.primary } }} - > - - - ) : ( - (card.isCommunity ? setConfirmTarget(card.community ?? null) : void handleInstallCurated(card))} - sx={{ color: c.text.secondary, '&:hover': { color: c.text.primary, bgcolor: c.bg.secondary } }} - > - - - )} - - - {card.publisher} - {card.installs !== null && ( - <> - - - - {formatInstallCount(card.installs)} - - - )} - - {card.description && ( - - {card.description} - - )} - - ); - })} - - - setConfirmTarget(null)} - onInstalled={(name) => { - setConfirmTarget(null); - void dispatch(fetchSkills()); - setSnackbar({ open: true, message: `Installed "${name}"`, severity: 'success' }); - }} - /> - - setSnackbar((p) => ({ ...p, open: false }))} - anchorOrigin={{ vertical: 'bottom', horizontal: 'center' }} - > - setSnackbar((p) => ({ ...p, open: false }))} sx={{ fontSize: '0.8125rem' }}> - {snackbar.message} - - - - ); -}; - -export default DirectorySkillsTab; diff --git a/frontend/src/app/pages/Directory/dialogs/CommunityInstallConfirm.tsx b/frontend/src/app/pages/Directory/dialogs/CommunityInstallConfirm.tsx deleted file mode 100644 index 1eacceed..00000000 --- a/frontend/src/app/pages/Directory/dialogs/CommunityInstallConfirm.tsx +++ /dev/null @@ -1,120 +0,0 @@ -import React, { useEffect, useRef, useState } 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 Button from '@mui/material/Button'; -import CircularProgress from '@mui/material/CircularProgress'; -import Alert from '@mui/material/Alert'; -import OpenInNewIcon from '@mui/icons-material/OpenInNew'; -import WarningAmberIcon from '@mui/icons-material/WarningAmber'; -import { useClaudeTokens } from '@/shared/styles/ThemeContext'; -import { installCommunitySkill, CommunitySkill, InstallDisclosure } from '@/shared/state/skillRegistrySlice'; - -interface Props { - skill: CommunitySkill | null; - onClose: () => void; - onInstalled: (name: string) => void; -} - -// The Directory's community-install gate: skills.sh code is unvetted, so the + never installs blind. -// Same disclosure contract as the old CommunitySkillsDialog: files + scripts shown before anything lands. -const CommunityInstallConfirm: React.FC = ({ skill, onClose, onInstalled }) => { - const c = useClaudeTokens(); - const [disclosure, setDisclosure] = useState(null); - const [busy, setBusy] = useState(false); - const [error, setError] = useState(null); - const previewSeq = useRef(0); - - useEffect(() => { - setDisclosure(null); - setError(null); - if (!skill) return; - const seq = ++previewSeq.current; - setBusy(true); - void installCommunitySkill(skill.source, skill.skillId, false) - .then((res) => { if (seq === previewSeq.current) setDisclosure(res.disclosure); }) - .catch((e: unknown) => { if (seq === previewSeq.current) setError(e instanceof Error ? e.message : 'Could not load skill'); }) - .finally(() => { if (seq === previewSeq.current) setBusy(false); }); - }, [skill]); - - const confirmInstall = async () => { - if (!skill) return; - setBusy(true); - setError(null); - try { - await installCommunitySkill(skill.source, skill.skillId, true); - onInstalled(disclosure?.name || skill.name); - } catch (e) { - setError(e instanceof Error ? e.message : 'Install failed'); - } finally { - setBusy(false); - } - }; - - return ( - - - Install community skill - - - {error && {error}} - {busy && !disclosure && } - {disclosure && ( - <> - {disclosure.name} - {disclosure.description && ( - {disclosure.description} - )} - - {skill?.source} - - - } 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} - - ))} - - - )} - - - - {disclosure && ( - - )} - - - ); -}; - -export default CommunityInstallConfirm;