[eric] import: only markdown becomes a bare skill, and skills always confirm (ENG-376)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01En8dRGsJPLrJCQBEkTH4Mp
This commit is contained in:
ciregenz
2026-08-20 22:22:49 -07:00
co-authored by Claude Fable 5
parent 5e0e89abe1
commit e8dc7c5fdd
6 changed files with 65 additions and 11 deletions
+3
View File
@@ -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)
+17
View File
@@ -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
@@ -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, (id: string) => 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<void>((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 {
@@ -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';
@@ -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);
});
@@ -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;
}