From 7d805cd1b8daf317370636388cf8828327065bf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=B8=80=E7=89=87=E4=B8=80=E7=89=87?= <3037678834@qq.com> Date: Sun, 8 Nov 2026 08:54:08 +0800 Subject: [PATCH] fix(skills): reject Windows device names when creating a skill (#7948) * fix(skills): reject Windows device names when creating a skill A skill name is a `custom/` directory component, but the hyphen-case grammar admits Windows reserved device names (`nul`, `con`, `com1`, ...). `skill_manage` create and `.skill` archive install accepted them: on Windows `mkdir("nul")` reports success without producing a directory and the first write inside it fails with FileNotFoundError; on Linux the install succeeds but the tree can never be copied, mounted or exported to a Windows host. Creation now rejects those names through `windows_incompatible_segment()`, the check uploads, sandbox writes, skill support files, thread IDs and agent names already use. Archive extraction also rejects member segments that cannot be created on every host. Reads and deletes stay permissive so a skill an older release created on a Linux host can still be inspected and removed. Fixes #7945 * test(skills): pin the extraction guard and the tool-level create wiring Address review findings on the skill-name portability change: - Isolate the extraction guard: an archive with a portable folder and a portable frontmatter name but a reserved support-file segment (`my-skill/references/nul.md`) now fails with the member-path error, so deleting the guard turns this suite red again. - Pin the `skill_manage` wiring: `create` of a device name raises through the tool, while `edit`/`delete` keep reaching the existing "not found" path. - Reword the `safe_extract_skill_archive` docstring: members before the offending one are already written into staging, so what the check guarantees is that the install never commits under `custom/`. --------- Co-authored-by: Willem Jiang --- CHANGELOG.md | 16 ++- .../harness/deerflow/skills/AGENTS.md | 1 + .../harness/deerflow/skills/installer.py | 11 ++ .../skills/storage/local_skill_storage.py | 4 + .../deerflow/skills/storage/skill_storage.py | 18 ++- .../deerflow/tools/skill_manage_tool.py | 5 +- backend/tests/test_skill_manage_tool.py | 44 +++++++ backend/tests/test_skill_name_portability.py | 121 ++++++++++++++++++ 8 files changed, 216 insertions(+), 4 deletions(-) create mode 100644 backend/tests/test_skill_name_portability.py diff --git a/CHANGELOG.md b/CHANGELOG.md index f2a477ba00..4316694677 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,7 +78,20 @@ This section accumulates work toward the **2.3.0** milestone threads. A group bound with its own `/connect` keeps its thread; a legacy group binding, which covers every group of the bot, starts a new one in each group. ([#7794]) - + +- **skills:** A custom skill name that is a Windows reserved device name + (`nul`, `con`, `com1`, ...) is now rejected when the skill is created — + `skill_manage` `create` and `.skill` archive install — on every platform. The + hyphen-case grammar admitted such names, and the skill name becomes a + directory under `custom/`, so creating one "succeeded" on Windows without + producing a directory and the first write inside it failed with + `FileNotFoundError`; on Linux the install succeeded but could never be + copied, mounted or exported to a Windows host. The archive path also refuses + member path segments that cannot be created on every host, matching support + file writes. Reading and deleting an existing skill stay permissive, so a + skill an older release created on a Linux host can still be inspected and + removed. ([#7945]) + - **sandbox:** Fresh lazy AIO runs with an explicit authorized skill view no longer create or acquire a sandbox before the first sandbox-backed tool call. Existing sandbox bindings and providers that require skill uploads @@ -16098,6 +16111,7 @@ with **180 merged pull requests** since the first 2.0 milestone tag. [#7831]: https://github.com/bytedance/deer-flow/pull/7831 [#7892]: https://github.com/bytedance/deer-flow/pull/7892 [#7907]: https://github.com/bytedance/deer-flow/pull/7907 +[#7945]: https://github.com/bytedance/deer-flow/issues/7945 [#7953]: https://github.com/bytedance/deer-flow/issues/7953 [#7958]: https://github.com/bytedance/deer-flow/pull/7958 [#7981]: https://github.com/bytedance/deer-flow/pull/7981 diff --git a/backend/packages/harness/deerflow/skills/AGENTS.md b/backend/packages/harness/deerflow/skills/AGENTS.md index 32481e9617..3c86af34e3 100644 --- a/backend/packages/harness/deerflow/skills/AGENTS.md +++ b/backend/packages/harness/deerflow/skills/AGENTS.md @@ -28,6 +28,7 @@ - `skills/describe.py` — `build_describe_skill_tool(catalog)` builds the `describe_skill` tool as a closure; `build_skill_search_setup(skills, enabled, ...)` produces a `SkillSearchSetup(describe_skill_tool, skill_names)` that is wired into both the LangGraph agent factory (`agent.py`) and the embedded client (`client.py`). - **Slash activation**: `/skill-name task` loads that enabled skill's `SKILL.md` for the current model call only. The resolver rejects leading whitespace, missing separators, reserved command syntax (`/new`, `/help`, `/bootstrap`, `/status`, `/models`, `/memory`, `/goal`, `/agent`, and the exact composer-only alias `/context compact`), disabled skills, and skills outside a custom agent's whitelist. A custom skill named `context` remains activatable for other task text. - **Installation**: `POST /api/skills/install` extracts .skill ZIP archive to custom/ directory +- **Name portability**: a skill name is a `custom/` directory component. Creation (`skill_manage` `create`, archive install) rejects Windows reserved device names via `windows_incompatible_segment()` on every platform, and extraction rejects member segments that cannot be created on every host; reads and deletes stay permissive for skills an older release created on Linux. Tests: `tests/test_skill_name_portability.py`. - **Managed integrations**: Lark/Feishu CLI support installs one global official `lark-*` pack as read-only `SkillCategory.INTEGRATION` entries under `/mnt/skills/integrations/lark-cli/...`; enabled flags, app configuration, and OAuth data remain per-user. Install resolves the newest `larksuite/cli` release from GitHub (`releases/latest`) at install time (falling back to a bottom-line pinned version if the lookup fails) rather than hard-coding the pack version; integrity relies on the official host + structural archive guards + a recorded hash of the effective installed tree after shared guidance injection (not a pinned archive-byte SHA, which GitHub does not keep stable). The Gateway image still installs a pinned `@larksuite/cli` binary, so `get_lark_integration_status` surfaces `latest_available_version` and `runtime_version_mismatch` for the UI. AIO installs additionally verify and publish official Linux amd64/arm64 binaries under `{DEER_FLOW_HOME}/integrations/lark-cli/sandbox-cli`, mounted read-only at `/mnt/integrations/lark-cli/runtime`; `/mnt/integrations/lark-cli/config` (app credentials, incl. the long-lived `appSecret`) is mounted **read-only** into the sandbox, its empty `config/locks` subdirectory is over-mounted writable for `lark-cli` coordination files, and `/mnt/integrations/lark-cli/data` (refreshable OAuth tokens) stays writable, all mapping to owner-only per-user directories. **Sandbox trust boundary:** the credential-bearing config and data dirs are still *readable* by arbitrary sandbox processes (the agent's `bash` tool, or code reached via prompt-injection in a tool result), so the app secret and tokens are exposed to sandbox-side code even though they never reach the browser — the read-only config mount only prevents in-sandbox tampering, not read/exfiltration. The sidecar credential-broker (Pattern B, issue #4338) is the fix that removes these plaintext mounts from sandbox execution: set `LARK_CLI_BROKER_IMAGE` on the provisioner (see `docker/lark-cli-broker/`) and the Gateway sends `provision_lark_cli_broker` on sandbox create. The provisioner then runs a `lark-cli-broker` sidecar that owns the per-user `config`/`config/locks`/`data` mounts (mounted into the **sidecar only**, at `/var/lark/{config,config/locks,data}` with only the nested locks mount writable) and serves the `lark-cli` command surface on Pod loopback (`http://127.0.0.1:8788`); a shim init container (`install-shim`) writes a forwarding `lark-cli` into the shared runtime `emptyDir`, so the sandbox gets `DEERFLOW_LARK_BROKER_URL` + a shim on PATH but **no** credential files. The on-PATH `bin/lark-cli` is a `/bin/sh` launcher that resolves a Python 3 interpreter and execs the Python shim body (`bin/lark-cli-shim.py`) beside it, so broker mode does not silently ENOEXEC on a sandbox image without a `#!/usr/bin/env python3`-resolvable interpreter — it fails loudly (exit 127, actionable message) and can be pinned with `DEERFLOW_LARK_BROKER_PYTHON`. The broker runs `lark-cli` in the sidecar's cwd and cannot see the sandbox filesystem, so cwd is intentionally **not** forwarded and file-I/O subcommands relative to the sandbox cwd are unsupported (command surface only). An optional `DEERFLOW_LARK_BROKER_DENY_SUBCOMMANDS` denylist (comma-separated command paths, matched in order through interleaved option values, forwarded from the provisioner) lets the broker refuse secret-dumping subcommands before spawning the binary. `lark_cli_env_overlay(broker=True)` therefore omits `LARKSUITE_CLI_CONFIG_DIR`/`DATA_DIR`; `sandbox_lark_broker_active()` caches confirmed provisioner capabilities for admission and fails closed on unknowns; bash overlays use the admitted sandbox's actual mode. Configured broker provisioning rejects direct credential mounts. `DEER_FLOW_LARK_CLI_SANDBOX_RUNTIME_DIR` supplies a validated, symlink-free pre-staged runtime for air-gapped deployments. For the remote provisioner (K8s), the runtime binary is otherwise provisioned by an optional init container + shared `emptyDir` (Pattern A): set `LARK_CLI_INIT_IMAGE` on the provisioner (see `docker/lark-cli-init/`) and the Gateway sends `provision_lark_cli_runtime` on sandbox create once the pack is installed, so remote installs skip the Gateway-side GitHub download entirely. Broker (Pattern B) supersedes the init-container binary (Pattern A) when both images are configured. `get_lark_integration_status(check_runtime=True)` surfaces `sandbox_runtime_mode` (`none` / `gateway-download` / `init-container` / `broker`) and `sandbox_runtime_ready` (remote modes read the provisioner `GET /api/capabilities`: `lark_cli_init_image` / `lark_cli_broker_image`), plus `sandbox_runtime_probed` separating a confirmed probe result from the conservative unprobed fallback (mutation responses carry a probed status built from a credential snapshot taken under the credential lock), so a green UI can't hide a chat-time `lark-cli: command not found`. Cheap status probes are explicitly not live-verified; users authorize or reconnect through the browser device-flow endpoints instead of running terminal commands. - **SkillScan**: `packages/harness/deerflow/skills/skillscan/` is the native deterministic scanner for `.skill` archives and agent-managed skill writes. It runs offline before the LLM scanner, emits structured findings (`rule_id`, `severity`, `file`, `line`, `message`, `remediation`, redacted `evidence` — category/analyzer are encoded in the `rule_id` prefix), blocks `CRITICAL`, and passes warning findings into `scan_skill_content()`. The moderation adapter must normalize both plain-text responses and LangChain Responses API text blocks before parsing the required JSON decision. `scan_archive_preflight()` / `scan_skill_dir()` are pure sync functions (dispatch off the event loop); `enforce_static_scan()` applies the blocking policy and the `skill_scan.enabled` kill switch. `skills/package_files.py` is the single definition of code files (`scripts/` members, code suffixes, extensionless `#!` files) and executable magic bytes; the installer, export guard, and SkillScan all import it, so do not re-derive either rule locally. Files that fail NUL-free UTF-8 decoding are treated as binaries unless they are code files: interpreters still run those, so they raise `package-undecodable-script` (HIGH) and are analyzed over a lossy decode, keeping one stray byte from hiding a file or downgrading a `CRITICAL` match; one with executable magic skips the text rules, which only misread its string tables. The Python instance-client signal deliberately follows only a one-level, same-scope evidence chain (PR #4265 review): a proven imported constructor bound to a simple name, optional name-to-name alias propagation, rebinding invalidation, and a constructor-supported outbound method or context-manager use; bare canonical-looking names never fall back to module identity. Nested scopes never inherit client handles and inherit only constructor aliases proven stable by a binding-only enclosing-scope prepass. Comprehensions, walrus-bearing statements, annotations, executable expressions inside complex binding targets, unsupported operations, and flows whose order cannot be decided produce no finding from this signal; representative false negatives are pinned by `test_python_declared_false_negatives_stay_unreported`. Compound bodies are walked from isolated copies so wrapping code in `if True:` is not a bypass: a part that cannot follow a sibling's rebinding (an `if` branch, the one `match` case, a `try` or loop body) applies its own bindings in source order, while a part that can (a loop or `try` `else`, a handler, a `finally`) starts with those names dropped; dispatch-time bindings (a loop target, `except ... as`, a `match` capture) are always dropped first, and nothing bound inside a body reaches the code after the statement. Copied scope entries, binding-only prepasses, and AST visits consume a deterministic work budget and the walk stops after its first sink. Budget or recursion exhaustion skips only this best-effort signal and retains deterministic findings already collected for the file. Do not add Semgrep/OpenGrep or YAML rule-engine dependencies to the core path; Phase 1 rule specs live in Python constants next to their analyzers in `skillscan/orchestrator.py`. - **SkillScan HTTP hosts**: recognize HTTP/HTTPS schemes case-insensitively in both URL regexes and prefix checks, without changing source evidence. Use URL authority parsing for lowercase hosts and bracketed IPv6 loopback; userinfo must never hide an external host. Malformed URLs remain outbound for exfiltration checks. Cleartext HTTP detection covers IPv6 literals; cloud-metadata hostname matching is case-insensitive. Coverage: `tests/test_skillscan_native.py`. diff --git a/backend/packages/harness/deerflow/skills/installer.py b/backend/packages/harness/deerflow/skills/installer.py index 5bfcc38606..8460d43aab 100644 --- a/backend/packages/harness/deerflow/skills/installer.py +++ b/backend/packages/harness/deerflow/skills/installer.py @@ -25,6 +25,7 @@ from deerflow.skills.security_static_scanner import ( scan_archive_preflight, skill_scan_enabled, ) +from deerflow.utils.host_paths import windows_incompatible_segment logger = logging.getLogger(__name__) @@ -128,6 +129,9 @@ def safe_extract_skill_archive( a huge number of tiny/empty members can be cheap to store yet still slow to extract, independent of total size). - Reject executable binaries (ELF/PE/Mach-O) by magic bytes. + - Reject member path segments that cannot be created on every host (Windows + reserved device names, trailing dots or spaces), so an unusable member + name fails the install instead of leaving a partially installed skill. Raises: ValueError: If unsafe members, executable binaries, entry count, or size limit exceeded. @@ -154,6 +158,13 @@ def safe_extract_skill_archive( continue normalized_name = posixpath.normpath(info.filename.replace("\\", "/")) + for segment in PurePosixPath(normalized_name).parts: + reason = windows_incompatible_segment(segment) + if reason is not None: + # Extraction creates one host path component per segment; a + # Windows device name cannot become one, and the same names are + # already refused for support-file writes and skill names. + raise ValueError(f"Archive member path is not portable to Windows: {info.filename!r} ({reason})") member_path = dest_root.joinpath(*PurePosixPath(normalized_name).parts) if not member_path.resolve().is_relative_to(dest_root): raise ValueError(f"Zip entry escapes destination: {info.filename!r}") diff --git a/backend/packages/harness/deerflow/skills/storage/local_skill_storage.py b/backend/packages/harness/deerflow/skills/storage/local_skill_storage.py index cbf3d13689..75b86657eb 100644 --- a/backend/packages/harness/deerflow/skills/storage/local_skill_storage.py +++ b/backend/packages/harness/deerflow/skills/storage/local_skill_storage.py @@ -205,6 +205,10 @@ class LocalSkillStorage(SkillStorage): raise ValueError(f"Invalid skill: {message}") if not skill_name or "/" in skill_name or "\\" in skill_name or ".." in skill_name: raise ValueError(f"Invalid skill name: {skill_name}") + # The skill name becomes ``custom/``; refuse names that cannot be + # created as a host path component (Windows reserved device names) so + # the install fails here instead of mid-write on the target host. + self.validate_skill_name(skill_name, for_creation=True) target = custom_dir / skill_name if target.exists(): diff --git a/backend/packages/harness/deerflow/skills/storage/skill_storage.py b/backend/packages/harness/deerflow/skills/storage/skill_storage.py index 66d08eb083..f1a23241e6 100644 --- a/backend/packages/harness/deerflow/skills/storage/skill_storage.py +++ b/backend/packages/harness/deerflow/skills/storage/skill_storage.py @@ -79,13 +79,27 @@ class SkillStorage(ABC): # ------------------------------------------------------------------ @staticmethod - def validate_skill_name(name: str) -> str: - """Validate and normalise a skill name; return the normalised form.""" + def validate_skill_name(name: str, *, for_creation: bool = False) -> str: + """Validate and normalise a skill name; return the normalised form. + + ``for_creation`` also refuses names that cannot be created as a host + path component. The hyphen-case grammar admits Windows reserved device + names (``nul``, ``con``, ``com1``, ...); a skill name is a directory + under ``custom/``, so creating one of those "succeeds" on Windows + without producing a directory and the first write inside it fails with + ``FileNotFoundError``. Only creation paths set this, so reading and + deleting a skill an older release created on a Linux host keeps + working. + """ normalized = name.strip() if not _SKILL_NAME_PATTERN.fullmatch(normalized): raise ValueError("Skill name must be hyphen-case using lowercase letters, digits, and hyphens only.") if len(normalized) > 64: raise ValueError("Skill name must be 64 characters or fewer.") + if for_creation: + reason = windows_incompatible_segment(normalized) + if reason is not None: + raise ValueError(f"Skill name {normalized!r} is not portable to Windows: {reason}.") return normalized @staticmethod diff --git a/backend/packages/harness/deerflow/tools/skill_manage_tool.py b/backend/packages/harness/deerflow/tools/skill_manage_tool.py index 7a63dc2180..c43ab681cd 100644 --- a/backend/packages/harness/deerflow/tools/skill_manage_tool.py +++ b/backend/packages/harness/deerflow/tools/skill_manage_tool.py @@ -234,7 +234,10 @@ async def _skill_manage_impl( replace: Replacement text for patch. expected_count: Optional expected number of replacements for patch. """ - name = SkillStorage.validate_skill_name(name) + # ``create`` also refuses a name that cannot become a directory component + # (Windows reserved device names); edits and deletes stay permissive so a + # skill an older release created still works. + name = SkillStorage.validate_skill_name(name, for_creation=action == "create") user_id = resolve_runtime_user_id(runtime) lock = _get_lock(user_id, name) thread_id = _get_thread_id(runtime) diff --git a/backend/tests/test_skill_manage_tool.py b/backend/tests/test_skill_manage_tool.py index c320d6df20..69e89dea8b 100644 --- a/backend/tests/test_skill_manage_tool.py +++ b/backend/tests/test_skill_manage_tool.py @@ -764,3 +764,47 @@ def test_skill_manage_later_call_does_not_overtake_a_parked_waiter(monkeypatch): assert storage.callers == ["queued", "late"] asyncio.run(scenario()) + + +def _patch_skill_manage_env(monkeypatch, tmp_path): + """Config, user paths and the scanner stubs every tool-level case needs.""" + skills_root = tmp_path / "skills" + config = _make_config(skills_root) + monkeypatch.setattr("deerflow.config.get_app_config", lambda: config) + monkeypatch.setattr("deerflow.skills.security_scanner.get_app_config", lambda: config) + from deerflow.config.paths import Paths + + monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: Paths(base_dir=tmp_path)) + monkeypatch.setattr("deerflow.config.paths._paths", None) + + async def _refresh(user_id: str): + return None + + monkeypatch.setattr(skill_manage_module, "refresh_user_skills_system_prompt_cache_async", _refresh) + monkeypatch.setattr(skill_manage_module, "scan_skill_content", lambda *args, **kwargs: _async_result("allow", "ok")) + return tmp_path / "users" / "default" / "skills" / "custom" + + +def test_skill_manage_create_rejects_windows_device_names(monkeypatch, tmp_path): + """Creation is the only action that must refuse a host-incompatible name.""" + user_custom = _patch_skill_manage_env(monkeypatch, tmp_path) + runtime = _make_runtime(user_id="default") + + with pytest.raises(ValueError, match="not portable to Windows"): + anyio.run(skill_manage_module.skill_manage_tool.coroutine, runtime, "create", "nul", _skill_content("nul")) + + # ``nul`` resolves to the NUL device on Windows, so assert on ``is_dir``. + assert not (user_custom / "nul").is_dir() + + +def test_skill_manage_edits_stay_permissive_for_device_named_skills(monkeypatch, tmp_path): + """An existing skill an older release created on Linux stays editable/deletable.""" + _patch_skill_manage_env(monkeypatch, tmp_path) + runtime = _make_runtime(user_id="default") + + # The name passes validation; the skill simply does not exist, which is a + # different error than the creation-time portability rejection. + with pytest.raises(FileNotFoundError, match="not found"): + anyio.run(skill_manage_module.skill_manage_tool.coroutine, runtime, "edit", "nul", _skill_content("nul")) + with pytest.raises(FileNotFoundError, match="not found"): + anyio.run(skill_manage_module.skill_manage_tool.coroutine, runtime, "delete", "nul") diff --git a/backend/tests/test_skill_name_portability.py b/backend/tests/test_skill_name_portability.py new file mode 100644 index 0000000000..ed89cf3db4 --- /dev/null +++ b/backend/tests/test_skill_name_portability.py @@ -0,0 +1,121 @@ +"""A skill name is a directory component, so creation rejects Windows device names.""" + +import asyncio +import zipfile +from pathlib import Path + +import pytest + +from deerflow.skills.storage.local_skill_storage import LocalSkillStorage +from deerflow.skills.storage.skill_storage import SkillStorage + +RESERVED_DEVICE_NAMES = ["nul", "con", "prn", "aux", "com1", "com9", "lpt1", "lpt9"] + + +def _build_archive(tmp_path: Path, name: str) -> Path: + archive = tmp_path / f"{name}.skill" + with zipfile.ZipFile(archive, "w") as zf: + zf.writestr( + f"{name}/SKILL.md", + f"---\nname: {name}\ndescription: portability probe\n---\n\n# probe\n", + ) + zf.writestr(f"{name}/references/note.md", "support file\n") + return archive + + +class TestValidateSkillNameForCreation: + @pytest.mark.parametrize("name", RESERVED_DEVICE_NAMES) + def test_creation_rejects_windows_device_names(self, name): + with pytest.raises(ValueError, match="not portable to Windows"): + SkillStorage.validate_skill_name(name, for_creation=True) + + @pytest.mark.parametrize("name", RESERVED_DEVICE_NAMES) + def test_reads_and_deletes_stay_permissive(self, name): + # A skill an older release created on a Linux host must stay readable + # and deletable; only creation refuses the name. + assert SkillStorage.validate_skill_name(name) == name + + @pytest.mark.parametrize("name", ["my-skill", "skill2", "a-b-c"]) + def test_creation_still_accepts_ordinary_names(self, name): + assert SkillStorage.validate_skill_name(name, for_creation=True) == name + + def test_creation_keeps_the_existing_shape_errors(self): + with pytest.raises(ValueError, match="hyphen-case"): + SkillStorage.validate_skill_name("Not Portable", for_creation=True) + with pytest.raises(ValueError, match="64 characters"): + SkillStorage.validate_skill_name("a" * 65, for_creation=True) + + +class TestArchiveInstallRejectsDeviceNames: + @pytest.mark.parametrize("name", RESERVED_DEVICE_NAMES) + def test_prepare_rejects_before_creating_the_target(self, tmp_path, name): + storage = LocalSkillStorage(host_path=str(tmp_path / "skills")) + archive = _build_archive(tmp_path, name) + custom_dir = tmp_path / "skills" / "custom" + custom_dir.mkdir(parents=True) + + with pytest.raises(ValueError, match="not portable to Windows"): + storage._prepare_skill_archive(archive, tmp_path / "extract", custom_dir, archive) + + # ``nul`` resolves to the NUL device on Windows, so ``exists()`` is + # true there even though nothing was created: assert on ``is_dir``. + assert not (custom_dir / name).is_dir() + + @pytest.mark.parametrize("name", ["my-skill"]) + def test_prepare_accepts_an_ordinary_name(self, tmp_path, name): + storage = LocalSkillStorage(host_path=str(tmp_path / "skills")) + archive = _build_archive(tmp_path, name) + custom_dir = tmp_path / "skills" / "custom" + custom_dir.mkdir(parents=True) + + skill_dir, installed_name, target = storage._prepare_skill_archive(archive, tmp_path / "extract", custom_dir, archive) + + assert installed_name == name + assert target == custom_dir / name + assert (skill_dir / "SKILL.md").is_file() + + def test_install_from_archive_rejects_a_device_name(self, tmp_path, monkeypatch): + async def _skip_scan(*_args, **_kwargs): + return [] + + monkeypatch.setattr("deerflow.skills.installer._scan_skill_archive_contents_or_raise", _skip_scan) + storage = LocalSkillStorage(host_path=str(tmp_path / "skills")) + archive = _build_archive(tmp_path, "nul") + + with pytest.raises(ValueError, match="not portable to Windows"): + asyncio.run(storage.ainstall_skill_from_archive(archive)) + + assert not (tmp_path / "skills" / "custom" / "nul").is_dir() + + def test_prepare_rejects_a_frontmatter_name_that_differs_from_the_folder(self, tmp_path): + # The installed directory is named by the frontmatter, not by the + # archive folder, so a portable folder name does not make the skill + # name portable. + storage = LocalSkillStorage(host_path=str(tmp_path / "skills")) + archive = tmp_path / "device-name.skill" + with zipfile.ZipFile(archive, "w") as zf: + zf.writestr("portable-folder/SKILL.md", "---\nname: nul\ndescription: probe\n---\n") + custom_dir = tmp_path / "skills" / "custom" + custom_dir.mkdir(parents=True) + + with pytest.raises(ValueError, match="not portable to Windows"): + storage._prepare_skill_archive(archive, tmp_path / "extract", custom_dir, archive) + + assert not (custom_dir / "nul").is_dir() + + def test_prepare_rejects_a_device_named_member_segment(self, tmp_path): + # Portable archive folder and portable frontmatter name: only a support + # file's segment is unusable, so this pins the extraction guard itself + # rather than the frontmatter name check. + storage = LocalSkillStorage(host_path=str(tmp_path / "skills")) + archive = tmp_path / "member-segment.skill" + with zipfile.ZipFile(archive, "w") as zf: + zf.writestr("my-skill/SKILL.md", "---\nname: my-skill\ndescription: probe\n---\n") + zf.writestr("my-skill/references/nul.md", "support file\n") + custom_dir = tmp_path / "skills" / "custom" + custom_dir.mkdir(parents=True) + + with pytest.raises(ValueError, match="Archive member path"): + storage._prepare_skill_archive(archive, tmp_path / "extract", custom_dir, archive) + + assert not (custom_dir / "my-skill").is_dir()