From e8dc7c5fddd9f4b6c590709a3e56ace121ad51d4 Mon Sep 17 00:00:00 2001 From: ciregenz Date: Thu, 20 Aug 2026 22:22:49 -0700 Subject: [PATCH] [eric] import: only markdown becomes a bare skill, and skills always confirm (ENG-376) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01En8dRGsJPLrJCQBEkTH4Mp --- backend/apps/swarm/closure.py | 3 ++ backend/tests/test_swarm_bundle.py | 17 ++++++++++ .../app/components/share/ImportEntryPoint.tsx | 12 ++----- .../src/app/components/share/ImportModal.tsx | 2 +- .../share/importNeedsConfirm.test.ts | 31 +++++++++++++++++++ .../components/share/importNeedsConfirm.ts | 11 +++++++ 6 files changed, 65 insertions(+), 11 deletions(-) create mode 100644 frontend/src/app/components/share/importNeedsConfirm.test.ts create mode 100644 frontend/src/app/components/share/importNeedsConfirm.ts diff --git a/backend/apps/swarm/closure.py b/backend/apps/swarm/closure.py index b09ec962..6010ef85 100644 --- a/backend/apps/swarm/closure.py +++ b/backend/apps/swarm/closure.py @@ -211,6 +211,9 @@ def stage_upload(raw: bytes, filename: str) -> tuple[str, Manifest, list[str]]: raise BundleError("this .swarm was made by a newer OpenSwarm; please update") return sandbox, manifest, warnings return stage_skill_from_zip(raw, filename, warnings) + # A .swarm that is not a zip is a broken or renamed bundle, and a screenshot or a PDF must never be reinterpreted as a skill to install (ENG-376). + if os.path.splitext(filename or "")[1].lower() not in (".md", ".markdown"): + raise BundleError("unrecognized file; expected a .swarm bundle or a .md skill") return p_stage_skill_from_markdown(raw, filename, warnings) diff --git a/backend/tests/test_swarm_bundle.py b/backend/tests/test_swarm_bundle.py index ffa977c4..d7c7f4be 100644 --- a/backend/tests/test_swarm_bundle.py +++ b/backend/tests/test_swarm_bundle.py @@ -620,3 +620,20 @@ def test_newer_format_version_rejected(skill_store): zf.writestr("entities/x/payload.json", json.dumps({"slug": "n", "name": "n", "content": "c"})) with pytest.raises(BundleError): closure.stage_upload(buf.getvalue(), "x.swarm") + + +def test_a_non_zip_swarm_is_refused_not_reread_as_a_skill(skill_store): + """ENG-376: any UTF-8 bytes named .swarm used to import as a markdown skill with no confirm.""" + with pytest.raises(BundleError): + closure.stage_upload(b"just some text", "notes.swarm") + + +def test_only_markdown_may_import_as_a_bare_skill(skill_store): + with pytest.raises(BundleError): + closure.stage_upload(b"hello", "notes.txt") + with pytest.raises(BundleError): + closure.stage_upload(b"\x89PNG\r\n", "shot.png") + sandbox, manifest, p_w = closure.stage_upload(b"# still fine", "Trick.markdown") + import shutil + shutil.rmtree(sandbox, ignore_errors=True) + assert manifest.root.type == EntityType.skill diff --git a/frontend/src/app/components/share/ImportEntryPoint.tsx b/frontend/src/app/components/share/ImportEntryPoint.tsx index 018eb96b..b01c39af 100644 --- a/frontend/src/app/components/share/ImportEntryPoint.tsx +++ b/frontend/src/app/components/share/ImportEntryPoint.tsx @@ -17,6 +17,7 @@ import ImportDigest, { DigestHandle } from './ImportDigest'; import ImportModal from './ImportModal'; import { importCommit, importPreflight } from './shareApi'; import { ImportPreflight } from './shareTypes'; +import { importNeedsConfirm } from './importNeedsConfirm'; import { DragVerdict, UNSUPPORTED_DROP_MESSAGE, firstImportable, judgeDrag, looksImportable } from './dragImportability'; export const IMPORT_OPEN_EVENT = 'openswarm:import-open'; @@ -28,15 +29,6 @@ const DEST: Record string | null> = { dashboard: (id) => `/dashboard/${id}`, }; -// A bundle needs a confirm only if it can run code (an app) or wants actions connected; everything else is inert data and imports straight away. -function needsConfirm(pf: ImportPreflight): boolean { - const s = pf.summary; - const hasApp = s.root.type === 'app' || s.includes.some((i) => i.type === 'app'); - const hasAction = s.requirements.some((r) => r.kind === 'mcp_action'); - const risky = !!pf.review && pf.review.verdict !== 'clean'; - return hasApp || hasAction || risky; -} - const delay = (ms: number) => new Promise((r) => setTimeout(r, ms)); const ImportEntryPoint: React.FC = () => { @@ -99,7 +91,7 @@ const ImportEntryPoint: React.FC = () => { setToast({ msg: e?.message || "We couldn't read this file.", sev: 'error' }); return; } - if (needsConfirm(pf)) { + if (importNeedsConfirm(pf)) { confirmRef.current = true; setConfirm(pf); } else { diff --git a/frontend/src/app/components/share/ImportModal.tsx b/frontend/src/app/components/share/ImportModal.tsx index 5987457b..b5121a8c 100644 --- a/frontend/src/app/components/share/ImportModal.tsx +++ b/frontend/src/app/components/share/ImportModal.tsx @@ -1,4 +1,4 @@ -// Confirmation surface shown only for bundles that carry something with a consequence (an app that runs code, or actions that must be connected). Safe bundles never reach here; the entry point auto-imports them. This is purely presentational: the entry point owns preflight, commit, and navigation. +// Confirmation surface shown only for bundles that carry something with a consequence (an app that runs code, a skill that rides every future turn, or actions that must be connected). Safe bundles never reach here; the entry point auto-imports them. This is purely presentational: the entry point owns preflight, commit, and navigation. import React from 'react'; import Box from '@mui/material/Box'; import Typography from '@mui/material/Typography'; diff --git a/frontend/src/app/components/share/importNeedsConfirm.test.ts b/frontend/src/app/components/share/importNeedsConfirm.test.ts new file mode 100644 index 00000000..8e2d28c6 --- /dev/null +++ b/frontend/src/app/components/share/importNeedsConfirm.test.ts @@ -0,0 +1,31 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; + +import { importNeedsConfirm } from './importNeedsConfirm'; +import type { ImportPreflight } from './shareTypes'; + +function preflight(rootType: string, includes: string[] = [], review: ImportPreflight['review'] = null): ImportPreflight { + return { + summary: { + root: { type: rootType, name: 'x' }, + includes: includes.map((type) => ({ type, name: type })), + requirements: [], + }, + review, + } as unknown as ImportPreflight; +} + +test('a skill never imports silently: it rides every future agent turn', () => { + assert.equal(importNeedsConfirm(preflight('skill')), true); + assert.equal(importNeedsConfirm(preflight('dashboard', ['skill'])), true); +}); + +test('inert data still imports straight away', () => { + assert.equal(importNeedsConfirm(preflight('dashboard')), false); + assert.equal(importNeedsConfirm(preflight('workflow', ['dashboard'])), false); +}); + +test('apps and failed reviews keep their confirm', () => { + assert.equal(importNeedsConfirm(preflight('app')), true); + assert.equal(importNeedsConfirm(preflight('dashboard', [], { verdict: 'flagged', findings: ['x'], scanned_files: [] })), true); +}); diff --git a/frontend/src/app/components/share/importNeedsConfirm.ts b/frontend/src/app/components/share/importNeedsConfirm.ts new file mode 100644 index 00000000..e62712bc --- /dev/null +++ b/frontend/src/app/components/share/importNeedsConfirm.ts @@ -0,0 +1,11 @@ +import type { ImportPreflight } from './shareTypes'; + +// A bundle needs a confirm when it can run code (an app), wants actions connected, failed review, or installs a skill: a skill lands in ~/.claude/skills and rides every future agent turn, so nothing may put one there without the user seeing what it is (ENG-376). Everything else is inert data and imports straight away. +export function importNeedsConfirm(pf: ImportPreflight): boolean { + const s = pf.summary; + const hasApp = s.root.type === 'app' || s.includes.some((i) => i.type === 'app'); + const hasSkill = s.root.type === 'skill' || s.includes.some((i) => i.type === 'skill'); + const hasAction = s.requirements.some((r) => r.kind === 'mcp_action'); + const risky = !!pf.review && pf.review.verdict !== 'clean'; + return hasApp || hasSkill || hasAction || risky; +}