Fix custom skill install permissions (#3241)

* Fix custom skill install permissions

* Fix skill upload test portability

* Keep custom skill writes sandbox readable

* Clear sandbox write bits on skill permissions

* Limit custom skill write permission updates

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

@@ -13,6 +13,7 @@ import stat
 import zipfile
 from pathlib import Path, PurePosixPath, PureWindowsPath
 
+from deerflow.skills.permissions import make_skill_tree_sandbox_readable
 from deerflow.skills.security_scanner import scan_skill_content
 
 logger = logging.getLogger(__name__)
@@ -139,6 +140,7 @@ def _move_staged_skill_into_reserved_target(staging_target: Path, target: Path)
         reserved = True
         for child in staging_target.iterdir():
             shutil.move(str(child), target / child.name)
+        make_skill_tree_sandbox_readable(target)
         installed = True
     except FileExistsError as e:
         raise SkillAlreadyExistsError(f"Skill '{target.name}' already exists") from e

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

@@ -0,0 +1,34 @@
+"""Filesystem permission helpers for installed skill trees."""
+
+import stat
+from pathlib import Path
+
+
+def make_skill_path_sandbox_readable(path: Path) -> None:
+    if path.is_symlink():
+        return
+    mode = stat.S_IMODE(path.stat().st_mode)
+    without_sandbox_write = mode & ~(stat.S_IWGRP | stat.S_IWOTH)
+    if path.is_dir():
+        path.chmod(without_sandbox_write | 0o555)
+    elif path.is_file():
+        path.chmod(without_sandbox_write | 0o444)
+
+
+def make_skill_tree_sandbox_readable(target: Path) -> None:
+    make_skill_path_sandbox_readable(target)
+    for path in target.rglob("*"):
+        make_skill_path_sandbox_readable(path)
+
+
+def make_skill_written_path_sandbox_readable(skill_root: Path, target: Path) -> None:
+    resolved_root = skill_root.resolve()
+    resolved_target = target.resolve()
+    resolved_target.relative_to(resolved_root)
+
+    make_skill_path_sandbox_readable(resolved_root)
+    current = resolved_root
+    for part in resolved_target.parent.relative_to(resolved_root).parts:
+        current = current / part
+        make_skill_path_sandbox_readable(current)
+    make_skill_path_sandbox_readable(resolved_target)

backend/packages/harness/deerflow/skills/storage/local_skill_storage.py

@@ -13,6 +13,7 @@ from datetime import UTC, datetime
 from pathlib import Path
 
 from deerflow.config.runtime_paths import resolve_path
+from deerflow.skills.permissions import make_skill_written_path_sandbox_readable
 from deerflow.skills.storage.skill_storage import SKILL_MD_FILE, SkillStorage
 from deerflow.skills.types import SkillCategory
 
@@ -90,6 +91,7 @@ class LocalSkillStorage(SkillStorage):
             tmp_file.write(content)
             tmp_path = Path(tmp_file.name)
         tmp_path.replace(target)
+        make_skill_written_path_sandbox_readable(self.get_custom_skill_dir(name), target)
 
     async def ainstall_skill_from_archive(self, archive_path: str | Path) -> dict:
         import zipfile

backend/tests/test_local_skill_storage_write.py

@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 import os
+import stat
 
 import pytest
 
@@ -43,6 +44,20 @@ def test_write_is_atomic_overwrite(tmp_path, storage):
     assert (tmp_path / "custom" / "demo-skill" / "SKILL.md").read_text() == "second"
 
 
+def test_write_makes_written_path_sandbox_readable(tmp_path, storage):
+    skill_dir = tmp_path / "custom" / "demo-skill"
+    skill_dir.mkdir(parents=True)
+    skill_dir.chmod(0o700)
+
+    storage.write_custom_skill("demo-skill", "references/ref.md", "# ref")
+
+    ref_dir = skill_dir / "references"
+    ref_file = ref_dir / "ref.md"
+    assert stat.S_IMODE(skill_dir.stat().st_mode) & 0o055 == 0o055
+    assert stat.S_IMODE(ref_dir.stat().st_mode) & 0o055 == 0o055
+    assert stat.S_IMODE(ref_file.stat().st_mode) & 0o044 == 0o044
+
+
 # ---------------------------------------------------------------------------
 # Empty / blank path
 # ---------------------------------------------------------------------------

backend/tests/test_skill_permissions.py

@@ -0,0 +1,63 @@
+import stat
+
+from deerflow.skills.permissions import make_skill_tree_sandbox_readable, make_skill_written_path_sandbox_readable
+
+
+def _mode(path):
+    return stat.S_IMODE(path.stat().st_mode)
+
+
+def test_skill_tree_readability_includes_hidden_paths_and_removes_sandbox_write(tmp_path):
+    root = tmp_path / "demo-skill"
+    hidden_dir = root / ".hidden"
+    scripts_dir = root / "scripts"
+    hidden_dir.mkdir(parents=True)
+    scripts_dir.mkdir()
+    env_file = root / ".env"
+    hidden_file = hidden_dir / ".secret"
+    script_file = scripts_dir / "run.sh"
+    env_file.write_text("secret", encoding="utf-8")
+    hidden_file.write_text("secret", encoding="utf-8")
+    script_file.write_text("#!/bin/sh\n", encoding="utf-8")
+
+    root.chmod(0o777)
+    hidden_dir.chmod(0o777)
+    scripts_dir.chmod(0o777)
+    env_file.chmod(0o666)
+    hidden_file.chmod(0o600)
+    script_file.chmod(0o777)
+
+    make_skill_tree_sandbox_readable(root)
+
+    assert _mode(root) == 0o755
+    assert _mode(hidden_dir) == 0o755
+    assert _mode(scripts_dir) == 0o755
+    assert _mode(env_file) == 0o644
+    assert _mode(hidden_file) == 0o644
+    assert _mode(script_file) == 0o755
+
+
+def test_written_path_readability_is_limited_to_written_path(tmp_path):
+    root = tmp_path / "demo-skill"
+    ref_dir = root / "references"
+    sibling_dir = root / "templates"
+    ref_dir.mkdir(parents=True)
+    sibling_dir.mkdir()
+    target = ref_dir / "guide.md"
+    sibling = sibling_dir / "note.md"
+    target.write_text("guide", encoding="utf-8")
+    sibling.write_text("note", encoding="utf-8")
+
+    root.chmod(0o700)
+    ref_dir.chmod(0o700)
+    target.chmod(0o600)
+    sibling_dir.chmod(0o700)
+    sibling.chmod(0o600)
+
+    make_skill_written_path_sandbox_readable(root, target)
+
+    assert _mode(root) == 0o755
+    assert _mode(ref_dir) == 0o755
+    assert _mode(target) == 0o644
+    assert _mode(sibling_dir) == 0o700
+    assert _mode(sibling) == 0o600

backend/tests/test_skills_custom_router.py

@@ -1,14 +1,18 @@
 import errno
 import json
+import stat
 import zipfile
+from io import BytesIO
 from pathlib import Path
 from types import SimpleNamespace
 
+from _router_auth_helpers import make_authed_test_app
 from fastapi import FastAPI
 from fastapi.testclient import TestClient
 
 from app.gateway.deps import get_config
 from app.gateway.routers import skills as skills_router
+from app.gateway.routers import uploads as uploads_router
 from deerflow.skills.storage import get_or_new_skill_storage
 from deerflow.skills.types import Skill
 
@@ -53,6 +57,15 @@ def _make_skill_archive(tmp_path: Path, name: str, content: str | None = None) -
     return archive
 
 
+def _make_skill_archive_bytes(name: str, content: str | None = None) -> bytes:
+    buffer = BytesIO()
+    skill_content = content or _skill_content(name)
+    with zipfile.ZipFile(buffer, "w") as zf:
+        zf.writestr(f"{name}/SKILL.md", skill_content)
+        zf.writestr(f"{name}/references/guide.md", "# Guide\n")
+    return buffer.getvalue()
+
+
 def test_install_skill_archive_runs_security_scan(monkeypatch, tmp_path):
     skills_root = tmp_path / "skills"
     (skills_root / "custom").mkdir(parents=True)
@@ -101,6 +114,65 @@ def test_install_skill_archive_runs_security_scan(monkeypatch, tmp_path):
     assert refresh_calls == ["refresh"]
 
 
+def test_uploaded_skill_archive_installs_sandbox_readable_tree(monkeypatch, tmp_path):
+    home = tmp_path / "home"
+    skills_root = tmp_path / "skills"
+    skills_root.mkdir()
+    refresh_calls = []
+
+    async def _scan(*args, **kwargs):
+        from deerflow.skills.security_scanner import ScanResult
+
+        return ScanResult(decision="allow", reason="ok")
+
+    async def _refresh():
+        refresh_calls.append("refresh")
+
+    config = SimpleNamespace(
+        skills=SimpleNamespace(get_skills_path=lambda: skills_root, container_path="/mnt/skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage"),
+        skill_evolution=SimpleNamespace(enabled=True, moderation_model_name=None),
+        uploads=SimpleNamespace(auto_convert_documents=False),
+    )
+    provider = SimpleNamespace(uses_thread_data_mounts=True)
+
+    monkeypatch.setenv("DEER_FLOW_HOME", str(home))
+    monkeypatch.setattr("deerflow.config.paths._paths", None)
+    monkeypatch.setattr(uploads_router, "get_sandbox_provider", lambda: provider)
+    monkeypatch.setattr("deerflow.skills.installer.scan_skill_content", _scan)
+    monkeypatch.setattr(skills_router, "refresh_skills_system_prompt_cache_async", _refresh)
+
+    app = make_authed_test_app()
+    app.state.config = config
+    app.dependency_overrides[get_config] = lambda: config
+    app.include_router(uploads_router.router)
+    app.include_router(skills_router.router)
+
+    thread_id = "thread-uploaded-skill"
+    archive_bytes = _make_skill_archive_bytes("uploaded-skill")
+
+    with TestClient(app) as client:
+        upload_response = client.post(
+            f"/api/threads/{thread_id}/uploads",
+            files=[("files", ("uploaded-skill.skill", archive_bytes, "application/octet-stream"))],
+        )
+        assert upload_response.status_code == 200
+        uploaded_file = upload_response.json()["files"][0]
+        uploaded_path = Path(uploaded_file["path"])
+        assert uploaded_path.is_file()
+
+        install_response = client.post("/api/skills/install", json={"thread_id": thread_id, "path": uploaded_file["virtual_path"]})
+
+    assert install_response.status_code == 200
+    assert install_response.json()["skill_name"] == "uploaded-skill"
+    installed_dir = skills_root / "custom" / "uploaded-skill"
+    nested_dir = installed_dir / "references"
+    assert stat.S_IMODE(installed_dir.stat().st_mode) & 0o055 == 0o055
+    assert stat.S_IMODE(nested_dir.stat().st_mode) & 0o055 == 0o055
+    assert stat.S_IMODE((installed_dir / "SKILL.md").stat().st_mode) & 0o044 == 0o044
+    assert stat.S_IMODE((nested_dir / "guide.md").stat().st_mode) & 0o044 == 0o044
+    assert refresh_calls == ["refresh"]
+
+
 def test_install_skill_archive_security_scan_block_returns_400(monkeypatch, tmp_path):
     skills_root = tmp_path / "skills"
     (skills_root / "custom").mkdir(parents=True)
@@ -175,6 +247,7 @@ def test_custom_skills_router_lifecycle(monkeypatch, tmp_path):
         )
         assert update_response.status_code == 200
         assert update_response.json()["description"] == "Edited skill"
+        assert stat.S_IMODE((custom_dir / "SKILL.md").stat().st_mode) & 0o044 == 0o044
 
         history_response = client.get("/api/skills/custom/demo-skill/history")
         assert history_response.status_code == 200
@@ -183,6 +256,7 @@ def test_custom_skills_router_lifecycle(monkeypatch, tmp_path):
         rollback_response = client.post("/api/skills/custom/demo-skill/rollback", json={"history_index": -1})
         assert rollback_response.status_code == 200
         assert rollback_response.json()["description"] == "Demo skill"
+        assert stat.S_IMODE((custom_dir / "SKILL.md").stat().st_mode) & 0o044 == 0o044
         assert refresh_calls == ["refresh", "refresh"]
 
 

backend/tests/test_skills_installer.py

@@ -198,6 +198,26 @@ class TestInstallSkillFromArchive:
         assert result["skill_name"] == "test-skill"
         assert (skills_root / "custom" / "test-skill" / "SKILL.md").exists()
 
+    def test_installed_skill_tree_is_readable_by_sandbox_mount(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/references/guide.md", "# Guide\n")
+        skills_root = tmp_path / "skills"
+        skills_root.mkdir()
+
+        get_or_new_skill_storage(skills_path=skills_root).install_skill_from_archive(zip_path)
+
+        installed_dir = skills_root / "custom" / "test-skill"
+        nested_dir = installed_dir / "references"
+        skill_file = installed_dir / "SKILL.md"
+        guide_file = nested_dir / "guide.md"
+
+        assert stat.S_IMODE(installed_dir.stat().st_mode) & 0o055 == 0o055
+        assert stat.S_IMODE(nested_dir.stat().st_mode) & 0o055 == 0o055
+        assert stat.S_IMODE(skill_file.stat().st_mode) & 0o044 == 0o044
+        assert stat.S_IMODE(guide_file.stat().st_mode) & 0o044 == 0o044
+
     def test_scans_skill_markdown_before_install(self, tmp_path, monkeypatch):
         zip_path = self._make_skill_zip(tmp_path)
         skills_root = tmp_path / "skills"