diff --git a/backend/packages/harness/deerflow/skills/installer.py b/backend/packages/harness/deerflow/skills/installer.py index 2e28f8c13..5c2aa59cb 100644 --- a/backend/packages/harness/deerflow/skills/installer.py +++ b/backend/packages/harness/deerflow/skills/installer.py @@ -20,6 +20,21 @@ logger = logging.getLogger(__name__) _PROMPT_INPUT_DIRS = {"references", "templates"} _PROMPT_INPUT_SUFFIXES = frozenset({".json", ".markdown", ".md", ".rst", ".txt", ".yaml", ".yml"}) +_CODE_SUFFIXES = frozenset({".bash", ".cjs", ".js", ".mjs", ".php", ".pl", ".ps1", ".py", ".rb", ".sh", ".ts", ".zsh"}) +# Full magics per variant — a shorter shared prefix would also match +# non-executable data files. +_EXECUTABLE_MAGIC_PREFIXES = ( + b"\x7fELF", # ELF + b"MZ", # PE/DOS + b"\xfe\xed\xfa\xce", # Mach-O 32-bit big-endian + b"\xfe\xed\xfa\xcf", # Mach-O 64-bit big-endian + b"\xce\xfa\xed\xfe", # Mach-O 32-bit little-endian + b"\xcf\xfa\xed\xfe", # Mach-O 64-bit little-endian + b"\xca\xfe\xba\xbe", # Mach-O fat binary big-endian + b"\xbe\xba\xfe\xca", # Mach-O fat binary little-endian + b"\xca\xfe\xba\xbf", # Mach-O fat64 binary big-endian + b"\xbf\xba\xfe\xca", # Mach-O fat64 binary little-endian +) class SkillAlreadyExistsError(ValueError): @@ -54,6 +69,11 @@ def is_symlink_member(info: zipfile.ZipInfo) -> bool: return stat.S_ISLNK(mode) +def is_executable_binary_prefix(prefix: bytes) -> bool: + """Detect ELF, PE, and Mach-O executables by magic bytes.""" + return prefix.startswith(_EXECUTABLE_MAGIC_PREFIXES) + + def should_ignore_archive_entry(path: Path) -> bool: """Return True for macOS metadata dirs and dotfiles.""" return path.name.startswith(".") or path.name == "__MACOSX" @@ -89,9 +109,10 @@ def safe_extract_skill_archive( - Reject absolute paths and directory traversal (..). - Skip symlink entries instead of materialising them. - Enforce a hard limit on total uncompressed size (zip bomb defence). + - Reject executable binaries (ELF/PE/Mach-O) by magic bytes. Raises: - ValueError: If unsafe members or size limit exceeded. + ValueError: If unsafe members, executable binaries, or size limit exceeded. """ dest_root = dest_path.resolve() total_written = 0 @@ -115,7 +136,11 @@ def safe_extract_skill_archive( continue with zip_ref.open(info) as src, member_path.open("wb") as dst: + first_chunk = True while chunk := src.read(65536): + if first_chunk and is_executable_binary_prefix(chunk): + raise ValueError(f"Archive contains executable binary member: {info.filename!r}") + first_chunk = False total_written += len(chunk) if total_written > max_total_size: raise ValueError("Skill archive is too large or appears highly compressed.") @@ -132,6 +157,32 @@ def _should_scan_support_file(rel_path: Path) -> bool: return bool(rel_path.parts) and rel_path.parts[0] in _PROMPT_INPUT_DIRS and rel_path.suffix.lower() in _PROMPT_INPUT_SUFFIXES +def _has_shebang(path: Path) -> bool: + try: + with path.open("rb") as f: + return f.read(2) == b"#!" + except OSError: + return False + + +def _is_code_file_by_name(rel_path: Path) -> bool: + """Pure name-based code classification: scripts/ members and code suffixes.""" + if _is_script_support_file(rel_path): + return True + return rel_path.suffix.lower() in _CODE_SUFFIXES + + +async def _is_code_file(path: Path, rel_path: Path) -> bool: + """Classify code files anywhere in the tree for the executable scan policy. + + Name checks are pure and stay on the event loop; only the shebang + sniff for extensionless files reads the file and is offloaded. + """ + if _is_code_file_by_name(rel_path): + return True + return not rel_path.suffix and await asyncio.to_thread(_has_shebang, path) + + def _move_staged_skill_into_reserved_target(staging_target: Path, target: Path) -> None: installed = False reserved = False @@ -190,10 +241,10 @@ async def _scan_skill_archive_contents_or_raise(skill_dir: Path, skill_name: str continue if path.name == "SKILL.md": raise SkillSecurityScanError(f"Security scan failed for skill '{skill_name}': nested SKILL.md is not allowed at {skill_name}/{rel_path.as_posix()}") - if not _should_scan_support_file(rel_path): - continue - - await _scan_skill_file_or_raise(skill_dir, path, skill_name, executable=_is_script_support_file(rel_path)) + if await _is_code_file(path, rel_path): + await _scan_skill_file_or_raise(skill_dir, path, skill_name, executable=True) + elif _should_scan_support_file(rel_path): + await _scan_skill_file_or_raise(skill_dir, path, skill_name, executable=False) def _run_async_install(coro): diff --git a/backend/tests/test_skills_installer.py b/backend/tests/test_skills_installer.py index 7c21bc288..e874628af 100644 --- a/backend/tests/test_skills_installer.py +++ b/backend/tests/test_skills_installer.py @@ -165,6 +165,64 @@ class TestSafeExtract: assert (dest / "my-skill" / "SKILL.md").exists() assert (dest / "my-skill" / "README.md").exists() + @pytest.mark.parametrize( + "magic", + [ + pytest.param(b"\x7fELF\x02\x01\x01\x00", id="elf"), + pytest.param(b"MZ\x90\x00\x03\x00\x00\x00", id="pe"), + pytest.param(b"\xfe\xed\xfa\xce\x00\x00\x00\x0c", id="mach-o-32-be"), + pytest.param(b"\xfe\xed\xfa\xcf\x00\x00\x00\x0c", id="mach-o-64-be"), + pytest.param(b"\xce\xfa\xed\xfe\x0c\x00\x00\x00", id="mach-o-32-le"), + pytest.param(b"\xcf\xfa\xed\xfe\x07\x00\x00\x01", id="mach-o-64-le"), + pytest.param(b"\xca\xfe\xba\xbe\x00\x00\x00\x02", id="mach-o-fat-be"), + pytest.param(b"\xbe\xba\xfe\xca\x02\x00\x00\x00", id="mach-o-fat-le"), + pytest.param(b"\xca\xfe\xba\xbf\x00\x00\x00\x02", id="mach-o-fat64-be"), + pytest.param(b"\xbf\xba\xfe\xca\x02\x00\x00\x00", id="mach-o-fat64-le"), + ], + ) + def test_rejects_executable_binary(self, tmp_path, magic): + zip_path = self._make_zip( + tmp_path, + { + "my-skill/SKILL.md": "---\nname: test\ndescription: x\n---\n# Test", + "my-skill/bin/tool": magic + b"\x00" * 64, + }, + ) + dest = tmp_path / "out" + dest.mkdir() + with zipfile.ZipFile(zip_path) as zf: + with pytest.raises(ValueError, match="executable binary"): + safe_extract_skill_archive(zf, dest) + + def test_allows_non_executable_binary_assets(self, tmp_path): + zip_path = self._make_zip( + tmp_path, + { + "my-skill/SKILL.md": "---\nname: test\ndescription: x\n---\n# Test", + "my-skill/assets/logo.png": b"\x89PNG\r\n\x1a\n" + b"\x00" * 32, + }, + ) + dest = tmp_path / "out" + dest.mkdir() + with zipfile.ZipFile(zip_path) as zf: + safe_extract_skill_archive(zf, dest) + assert (dest / "my-skill" / "assets" / "logo.png").exists() + + def test_allows_asset_sharing_a_partial_magic_prefix(self, tmp_path): + """Only full 4-byte magics are executable; \\xfe\\xed\\xfa + other byte is data.""" + zip_path = self._make_zip( + tmp_path, + { + "my-skill/SKILL.md": "---\nname: test\ndescription: x\n---\n# Test", + "my-skill/assets/blob.bin": b"\xfe\xed\xfa\x00" + b"\x00" * 32, + }, + ) + dest = tmp_path / "out" + dest.mkdir() + with zipfile.ZipFile(zip_path) as zf: + safe_extract_skill_archive(zf, dest) + assert (dest / "my-skill" / "assets" / "blob.bin").exists() + # --------------------------------------------------------------------------- # install_skill_from_archive (full integration) @@ -286,6 +344,93 @@ class TestInstallSkillFromArchive: ] assert all("secret" not in call["content"] for call in calls) + def test_scans_code_files_anywhere_in_tree(self, tmp_path, monkeypatch): + zip_path = tmp_path / "test-skill.skill" + with zipfile.ZipFile(zip_path, "w") as zf: + zf.writestr("test-skill/SKILL.md", "---\nname: test-skill\ndescription: A test skill\n---\n\n# test-skill\n") + zf.writestr("test-skill/helper.py", "import os\nprint('root code')\n") + zf.writestr("test-skill/lib/util.sh", "echo lib\n") + zf.writestr("test-skill/bin/tool", "#!/usr/bin/env python3\nprint('extensionless')\n") + zf.writestr("test-skill/assets/logo.png", b"\x89PNG\r\n\x1a\n") + zf.writestr("test-skill/assets/data.txt", "just data\n") + skills_root = tmp_path / "skills" + skills_root.mkdir() + calls = [] + + async def _scan(content, *, executable, location): + calls.append({"executable": executable, "location": location}) + return ScanResult(decision="allow", reason="ok") + + monkeypatch.setattr("deerflow.skills.installer.scan_skill_content", _scan) + + get_or_new_skill_storage(skills_path=skills_root).install_skill_from_archive(zip_path) + + assert {"executable": True, "location": "test-skill/helper.py"} in calls + assert {"executable": True, "location": "test-skill/lib/util.sh"} in calls + assert {"executable": True, "location": "test-skill/bin/tool"} in calls + scanned_locations = {call["location"] for call in calls} + assert "test-skill/assets/logo.png" not in scanned_locations + assert "test-skill/assets/data.txt" not in scanned_locations + + def test_shebang_sniff_only_reads_extensionless_files(self, tmp_path, monkeypatch): + """Suffix/scripts classification is name-based; only extensionless files are opened.""" + import deerflow.skills.installer as installer_module + + zip_path = tmp_path / "test-skill.skill" + with zipfile.ZipFile(zip_path, "w") as zf: + zf.writestr("test-skill/SKILL.md", "---\nname: test-skill\ndescription: A test skill\n---\n\n# test-skill\n") + zf.writestr("test-skill/helper.py", "print('code')\n") + zf.writestr("test-skill/scripts/run.sh", "#!/bin/sh\necho ok\n") + zf.writestr("test-skill/bin/tool", "#!/usr/bin/env python3\nprint('extensionless')\n") + zf.writestr("test-skill/assets/data.txt", "just data\n") + skills_root = tmp_path / "skills" + skills_root.mkdir() + sniffed = [] + original_has_shebang = installer_module._has_shebang + + def _tracking_has_shebang(path): + sniffed.append(path.name) + return original_has_shebang(path) + + monkeypatch.setattr(installer_module, "_has_shebang", _tracking_has_shebang) + + get_or_new_skill_storage(skills_path=skills_root).install_skill_from_archive(zip_path) + + assert sniffed == ["tool"] + + def test_code_file_outside_scripts_warn_prevents_install(self, tmp_path, monkeypatch): + zip_path = tmp_path / "test-skill.skill" + with zipfile.ZipFile(zip_path, "w") as zf: + zf.writestr("test-skill/SKILL.md", "---\nname: test-skill\ndescription: A test skill\n---\n\n# test-skill\n") + zf.writestr("test-skill/lib/run.py", "import os\nos.system('whoami')\n") + skills_root = tmp_path / "skills" + skills_root.mkdir() + + async def _scan(*args, executable, **kwargs): + if executable: + return ScanResult(decision="warn", reason="code needs review") + return ScanResult(decision="allow", reason="ok") + + monkeypatch.setattr("deerflow.skills.installer.scan_skill_content", _scan) + + with pytest.raises(SkillSecurityScanError, match="rejected executable.*code needs review"): + get_or_new_skill_storage(skills_path=skills_root).install_skill_from_archive(zip_path) + + assert not (skills_root / "custom" / "test-skill").exists() + + def test_executable_binary_prevents_install(self, tmp_path): + zip_path = tmp_path / "test-skill.skill" + with zipfile.ZipFile(zip_path, "w") as zf: + zf.writestr("test-skill/SKILL.md", "---\nname: test-skill\ndescription: A test skill\n---\n\n# test-skill\n") + zf.writestr("test-skill/bin/tool", b"\x7fELF\x02\x01\x01\x00" + b"\x00" * 64) + skills_root = tmp_path / "skills" + skills_root.mkdir() + + with pytest.raises(ValueError, match="executable binary"): + get_or_new_skill_storage(skills_path=skills_root).install_skill_from_archive(zip_path) + + assert not (skills_root / "custom" / "test-skill").exists() + def test_nested_skill_markdown_prevents_install(self, tmp_path): zip_path = tmp_path / "test-skill.skill" with zipfile.ZipFile(zip_path, "w") as zf: