fix(skills): reject colon in zip member names to close NTFS ADS smuggling gap (#4236)

Neither is_unsafe_zip_member (installer.py) nor its duplicated check in
skillscan/orchestrator.py rejected a colon in a zip member name. On
Windows/NTFS, a name like scripts/run.sh:hidden.txt addresses an
Alternate Data Stream on run.sh instead of creating a new file, so the
hidden content is invisible to Path.rglob()/os.walk()-based scanning in
both the deterministic static scanner and the extracted-file content
scanner, while still landing genuinely on disk. Reject any colon in a
zip member's relative path outright in both files; a colon has no
legitimate use there since zip entries use forward slashes and a real
Windows drive prefix is already caught by the existing absolute-path
check.

backend/packages/harness/deerflow/skills/installer.py

@@ -62,7 +62,21 @@ class SkillSecurityScanError(ValueError):
 
 
 def is_unsafe_zip_member(info: zipfile.ZipInfo) -> bool:
-    """Return True if the zip member path is absolute or attempts directory traversal."""
+    """Return True if the zip member path is absolute, attempts directory
+    traversal, or contains a colon.
+
+    A colon has no legitimate use in a relative archive member path — zip
+    entries always use ``/`` separators, and a real Windows drive prefix
+    (``C:\\...``) is already rejected above as absolute. But on Windows/NTFS,
+    a colon anywhere else in a path (e.g. ``scripts/run.sh:hidden.txt``)
+    addresses an Alternate Data Stream on the preceding path component
+    instead of creating a new file: it silently attaches extra content to
+    ``scripts/run.sh`` rather than creating a sibling file. That stream is
+    invisible to ``Path.rglob()`` / ``os.walk()``-based listing, so it would
+    let an archive smuggle content past directory-based security scanning
+    while the content still lands on disk. Reject outright rather than
+    trying to allow-list "safe" colon positions.
+    """
     name = info.filename
     if not name:
         return False
@@ -76,6 +90,8 @@ def is_unsafe_zip_member(info: zipfile.ZipInfo) -> bool:
         return True
     if ".." in path.parts:
         return True
+    if ":" in name:
+        return True
     return False
 
 

backend/packages/harness/deerflow/skills/skillscan/orchestrator.py

@@ -43,6 +43,12 @@ _MAX_ARCHIVE_MEMBERS = 4096
 _SPECS = [
     RuleSpec("package-path-traversal", "CRITICAL", "Archive member path traverses outside the skill root.", "Remove parent-directory traversal from the package path."),
     RuleSpec("package-absolute-path", "CRITICAL", "Archive member path is absolute.", "Use relative paths inside the skill archive."),
+    RuleSpec(
+        "package-ads-stream-name",
+        "CRITICAL",
+        "Archive member path contains a colon, which on Windows/NTFS addresses an alternate data stream hidden from directory listing.",
+        "Remove colons from archive member paths.",
+    ),
     RuleSpec("package-symlink", "HIGH", "Package contains a symlink entry.", "Remove symlinks from the skill package."),
     RuleSpec("package-nested-skill-md", "CRITICAL", "Package contains a nested SKILL.md file.", "Keep exactly one SKILL.md at the skill root."),
     RuleSpec("package-oversized-total", "CRITICAL", "Package total uncompressed size exceeds the limit.", "Remove large files or split assets out of the skill package."),
@@ -246,6 +252,8 @@ def _scan_archive_member_metadata(info: zipfile.ZipInfo, normalized: str) -> lis
         findings.append(_finding("package-absolute-path", file=normalized, evidence=info.filename))
     elif _archive_member_traverses(info.filename):
         findings.append(_finding("package-path-traversal", file=normalized, evidence=info.filename))
+    elif _archive_member_has_colon(info.filename):
+        findings.append(_finding("package-ads-stream-name", file=normalized, evidence=info.filename))
     if _is_symlink_member(info):
         findings.append(_finding("package-symlink", file=normalized, evidence=info.filename))
     parts = PurePosixPath(normalized).parts
@@ -545,6 +553,18 @@ def _archive_member_traverses(name: str) -> bool:
     return ".." in PurePosixPath(name.replace("\\", "/")).parts
 
 
+def _archive_member_has_colon(name: str) -> bool:
+    # A colon has no legitimate use in a relative archive member path (zip
+    # entries use ``/`` separators; a real Windows drive prefix is already
+    # caught by ``_archive_member_is_absolute``). On Windows/NTFS a colon
+    # elsewhere in the path addresses an Alternate Data Stream on the
+    # preceding path component (e.g. ``scripts/run.sh:hidden.txt`` attaches
+    # hidden content to ``run.sh`` instead of creating a sibling file), and
+    # that stream is invisible to directory-listing-based scanning. Reject
+    # outright rather than trying to allow-list "safe" colon positions.
+    return ":" in name
+
+
 def _is_symlink_member(info: zipfile.ZipInfo) -> bool:
     return stat.S_ISLNK(info.external_attr >> 16)
 

backend/tests/test_skills_installer.py

@@ -35,6 +35,14 @@ class TestIsUnsafeZipMember:
         info = zipfile.ZipInfo("C:\\Windows\\system32\\drivers\\etc\\hosts")
         assert is_unsafe_zip_member(info) is True
 
+    def test_colon_in_member_name_ntfs_ads(self):
+        """A colon after the first path component addresses a Windows NTFS
+        Alternate Data Stream (e.g. ``run.sh:hidden.txt`` hides content inside
+        ``run.sh`` instead of creating a new file), invisible to rglob/os.walk
+        based scanning. Must be rejected outright."""
+        info = zipfile.ZipInfo("my-skill/scripts/run.sh:hidden.txt")
+        assert is_unsafe_zip_member(info) is True
+
     def test_dotdot_traversal(self):
         info = zipfile.ZipInfo("foo/../../../etc/passwd")
         assert is_unsafe_zip_member(info) is True
@@ -139,6 +147,21 @@ class TestSafeExtract:
             with pytest.raises(ValueError, match="unsafe"):
                 safe_extract_skill_archive(zf, dest)
 
+    def test_rejects_ntfs_ads_colon_member(self, tmp_path):
+        """A zip member named like ``scripts/run.sh:hidden.txt`` is an NTFS
+        Alternate-Data-Stream address, not a nested path. Extraction must
+        reject the whole archive instead of silently attaching hidden
+        content to ``run.sh``."""
+        zip_path = tmp_path / "ads.zip"
+        with zipfile.ZipFile(zip_path, "w") as zf:
+            zf.writestr("my-skill/scripts/run.sh", "#!/bin/sh\necho ok\n")
+            zf.writestr("my-skill/scripts/run.sh:hidden.txt", "HIDDEN_PAYLOAD_MARKER")
+        dest = tmp_path / "out"
+        dest.mkdir()
+        with zipfile.ZipFile(zip_path) as zf:
+            with pytest.raises(ValueError, match="unsafe"):
+                safe_extract_skill_archive(zf, dest)
+
     def test_skips_symlinks(self, tmp_path):
         zip_path = tmp_path / "sym.zip"
         with zipfile.ZipFile(zip_path, "w") as zf:
@@ -553,6 +576,42 @@ class TestInstallSkillFromArchive:
 
         assert not (skills_root / "custom" / "test-skill").exists()
 
+    def test_ntfs_ads_smuggling_prevented(self, tmp_path, monkeypatch):
+        """End-to-end regression: an archive member name like
+        ``scripts/run.sh:hidden.txt`` addresses a Windows NTFS Alternate Data
+        Stream rather than a nested file. It must be rejected by the archive
+        preflight scan before extraction — not installed with its payload
+        left invisible to directory-based scanning."""
+        marker = "HIDDEN_ADS_PAYLOAD_MARKER_TEST"
+        zip_path = tmp_path / "ads-skill.skill"
+        with zipfile.ZipFile(zip_path, "w") as zf:
+            zf.writestr("ads-skill/SKILL.md", "---\nname: ads-skill\ndescription: A test skill\n---\n\n# ads-skill\n")
+            zf.writestr("ads-skill/scripts/run.sh", "#!/bin/sh\necho ok\n")
+            zf.writestr("ads-skill/scripts/run.sh:hidden.txt", marker)
+        skills_root = tmp_path / "skills"
+        skills_root.mkdir()
+        llm_calls = []
+
+        async def _scan(*args, **kwargs):
+            llm_calls.append({"args": args, "kwargs": kwargs})
+            return ScanResult(decision="allow", reason="ok")
+
+        monkeypatch.setattr("deerflow.skills.installer.scan_skill_content", _scan)
+
+        with pytest.raises(SkillSecurityScanError) as excinfo:
+            get_or_new_skill_storage(skills_path=skills_root).install_skill_from_archive(zip_path)
+
+        assert "Static security scan blocked" in str(excinfo.value)
+        assert excinfo.value.findings
+        assert excinfo.value.findings[0]["rule_id"] == "package-ads-stream-name"
+        assert llm_calls == []
+        assert not (skills_root / "custom" / "ads-skill").exists()
+        # The marker must not have leaked into skills_root anywhere (e.g. a
+        # partially-extracted temp dir surviving past cleanup).
+        for path in skills_root.rglob("*"):
+            if path.is_file():
+                assert marker not in path.read_text(encoding="utf-8", errors="ignore")
+
     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:

backend/tests/test_skillscan_native.py

@@ -174,6 +174,25 @@ def test_archive_preflight_reports_package_findings(tmp_path: Path) -> None:
     assert result["blocked"] is True
 
 
+def test_archive_preflight_rejects_ntfs_ads_colon_member(tmp_path: Path) -> None:
+    """A member name like ``scripts/run.sh:hidden.txt`` addresses a Windows
+    NTFS Alternate Data Stream on ``run.sh`` rather than a nested file. Such
+    a stream is invisible to rglob/os.walk-based scanning once extracted, so
+    the archive-level preflight must block it before extraction ever runs."""
+    archive = tmp_path / "demo-skill.skill"
+    with zipfile.ZipFile(archive, "w") as zf:
+        zf.writestr("demo-skill/SKILL.md", "---\nname: demo-skill\ndescription: Demo skill\n---\n")
+        zf.writestr("demo-skill/scripts/run.sh", "#!/bin/sh\necho ok\n")
+        zf.writestr("demo-skill/scripts/run.sh:hidden.txt", "HIDDEN_PAYLOAD_MARKER")
+
+    result = scan_archive_preflight(archive)
+
+    finding = _finding_by_rule(result["findings"], "package-ads-stream-name")
+    assert finding["severity"] == "CRITICAL"
+    assert finding["file"] == "demo-skill/scripts/run.sh:hidden.txt"
+    assert result["blocked"] is True
+
+
 def test_nested_zip_with_executable_member_escalates_to_critical(tmp_path: Path) -> None:
     archive = tmp_path / "demo-skill.skill"
     with zipfile.ZipFile(archive, "w") as zf: