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/<name>` 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 <willem.jiang@gmail.com>

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

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}")

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/<name>``; 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():

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

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)

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")

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()