fix(skills): resolve the user-scoped install content scan from its own config (#5703)

* fix(skills): resolve the user-scoped install scan from its own config

UserScopedSkillStorage.ainstall_skill_from_archive re-implements the parent
body to redirect the install target into the per-user custom root, and the
copy dropped the app_config argument. _scan_skill_archive_contents_or_raise
therefore fell back to the process-global get_app_config() for the static
scan, while the archive preflight it inherits still read self._app_config --
so the two gates of a single install could consult different configs.

The divergence is reachable whenever the storage outlives a config edit:
DeerFlowClient snapshots get_app_config() at construction, and the Gateway
hands its per-request get_config() to get_or_new_user_skill_storage, while
get_app_config() hot-reloads a later edit of config.yaml. With the storage's
config enabling skill_scan and the global disabling it, the preflight ran,
the content scan silently skipped, and a CRITICAL archive installed.

* fix(skills): thread app_config into the per-file LLM scan

_scan_skill_archive_contents_or_raise takes app_config and uses it for the
static gate, but never forwarded it to _scan_skill_file_or_raise, so the
per-file LLM scan fell back to the process-global get_app_config() for
skill_evolution.moderation_model_name and the model it constructs. Under
the stale-snapshot scenario the content-scan fix addresses, the two halves
of one install still read two different configs.

Raised in review of #5703 by willem-bd. The gateway's own skill-write
routes already pass app_config to scan_skill_content; this brings the
install path in line.

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

@@ -228,7 +228,7 @@ def _findings_for_file(findings: list[StaticFinding], rel_path: str) -> list[Sta
     return [finding for finding in findings if finding.get("file") in {rel_path, None}]
 
 
-async def _scan_skill_file_or_raise(skill_dir: Path, path: Path, skill_name: str, *, executable: bool, static_findings: list[StaticFinding] | None = None) -> None:
+async def _scan_skill_file_or_raise(skill_dir: Path, path: Path, skill_name: str, *, executable: bool, static_findings: list[StaticFinding] | None = None, app_config=None) -> None:
     rel_path = path.relative_to(skill_dir).as_posix()
     location = f"{skill_name}/{rel_path}"
     try:
@@ -237,7 +237,7 @@ async def _scan_skill_file_or_raise(skill_dir: Path, path: Path, skill_name: str
         raise SkillSecurityScanError(f"Security scan failed for skill '{skill_name}': {location} must be valid UTF-8") from e
 
     try:
-        result = await scan_skill_content(content, executable=executable, location=location, static_findings=static_findings or [])
+        result = await scan_skill_content(content, executable=executable, location=location, app_config=app_config, static_findings=static_findings or [])
     except Exception as e:
         raise SkillSecurityScanError(f"Security scan failed for {location}: {e}") from e
 
@@ -289,7 +289,7 @@ async def _scan_skill_archive_contents_or_raise(skill_dir: Path, skill_name: str
     static_findings = await _scan_static_skill_archive_or_raise(skill_dir, skill_name, app_config=app_config)
 
     skill_md = skill_dir / "SKILL.md"
-    await _scan_skill_file_or_raise(skill_dir, skill_md, skill_name, executable=False, static_findings=_findings_for_file(static_findings, "SKILL.md"))
+    await _scan_skill_file_or_raise(skill_dir, skill_md, skill_name, executable=False, static_findings=_findings_for_file(static_findings, "SKILL.md"), app_config=app_config)
 
     for path in await asyncio.to_thread(_collect_scannable_files, skill_dir):
         rel_path = path.relative_to(skill_dir)
@@ -305,6 +305,7 @@ async def _scan_skill_archive_contents_or_raise(skill_dir: Path, skill_name: str
                 skill_name,
                 executable=True,
                 static_findings=_findings_for_file(static_findings, rel_path_posix),
+                app_config=app_config,
             )
         elif _should_scan_support_file(rel_path):
             await _scan_skill_file_or_raise(
@@ -313,6 +314,7 @@ async def _scan_skill_archive_contents_or_raise(skill_dir: Path, skill_name: str
                 skill_name,
                 executable=False,
                 static_findings=_findings_for_file(static_findings, rel_path_posix),
+                app_config=app_config,
             )
     return static_findings
 

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

@@ -340,7 +340,7 @@ class UserScopedSkillStorage(LocalSkillStorage):
         try:
             skill_dir, skill_name, target = await asyncio.to_thread(self._prepare_skill_archive, path, Path(tmp), custom_dir, archive_path)
 
-            await _scan_skill_archive_contents_or_raise(skill_dir, skill_name)
+            await _scan_skill_archive_contents_or_raise(skill_dir, skill_name, app_config=self._app_config)
 
             await asyncio.to_thread(self._commit_skill_install, skill_dir, skill_name, custom_dir, target)
             logger.info("Skill %r installed to %s for user %s", skill_name, target, self._user_id)

backend/tests/test_skills_installer.py

@@ -455,7 +455,7 @@ class TestInstallSkillFromArchive:
         skills_root.mkdir()
         calls = []
 
-        async def _scan(content, *, executable, location, static_findings=None):
+        async def _scan(content, *, executable, location, app_config=None, static_findings=None):
             calls.append({"content": content, "executable": executable, "location": location})
             return ScanResult(decision="allow", reason="ok")
 
@@ -485,7 +485,7 @@ class TestInstallSkillFromArchive:
         skills_root.mkdir()
         calls = []
 
-        async def _scan(content, *, executable, location, static_findings=None):
+        async def _scan(content, *, executable, location, app_config=None, static_findings=None):
             calls.append({"content": content, "executable": executable, "location": location})
             return ScanResult(decision="allow", reason="ok")
 
@@ -530,7 +530,7 @@ class TestInstallSkillFromArchive:
         skills_root.mkdir()
         calls = []
 
-        async def _scan(content, *, executable, location, static_findings=None):
+        async def _scan(content, *, executable, location, app_config=None, static_findings=None):
             calls.append({"executable": executable, "location": location})
             return ScanResult(decision="allow", reason="ok")
 

backend/tests/test_user_scoped_skill_storage.py

@@ -3,13 +3,18 @@
 from __future__ import annotations
 
 import stat
+import zipfile
 from pathlib import Path
+from types import SimpleNamespace
 from unittest.mock import patch
 
 import pytest
 
 from deerflow.config.paths import Paths
+from deerflow.skills.installer import SkillSecurityScanError
+from deerflow.skills.security_scanner import ScanResult
 from deerflow.skills.storage import reset_skill_storage, reset_user_skill_storage
+from deerflow.skills.storage.local_skill_storage import LocalSkillStorage
 from deerflow.skills.storage.user_scoped_skill_storage import UserScopedSkillStorage
 from deerflow.skills.types import SkillCategory
 
@@ -698,3 +703,132 @@ class TestEnabledSkillsByConfigCacheBounded:
         assert (id(new_cfg), "user-new") in kept
         assert (id(configs[2]), "user-2") not in kept, "LRU should have been evicted"
         assert len(prompt_module._enabled_skills_by_config_cache) == 4
+
+
+def _skills_root(base: Path) -> Path:
+    root = base / "skills"
+    root.mkdir()
+    (root / "public").mkdir()
+    (root / "custom").mkdir()
+    return root
+
+
+def _storage_config(skills_root: Path, *, scan_enabled: bool) -> SimpleNamespace:
+    """An app_config carrying the ``skill_scan.enabled`` kill switch."""
+    return SimpleNamespace(
+        skills=SimpleNamespace(
+            get_skills_path=lambda: skills_root,
+            container_path="/mnt/skills",
+            use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage",
+        ),
+        skill_scan=SimpleNamespace(enabled=scan_enabled),
+    )
+
+
+def _malicious_archive(directory: Path, skill_name: str = "evil-skill") -> Path:
+    """A well-formed archive whose SKILL.md carries CRITICAL private-key material."""
+    return _archive(directory, skill_name, extra="-----BEGIN RSA PRIVATE KEY-----\nMIIEowIBAAKCAQEA\n-----END RSA PRIVATE KEY-----\n")
+
+
+def _archive(directory: Path, skill_name: str = "benign-skill", extra: str = "") -> Path:
+    """A well-formed archive containing one SKILL.md."""
+    path = directory / f"{skill_name}.skill"
+    with zipfile.ZipFile(path, "w") as zf:
+        zf.writestr(
+            f"{skill_name}/SKILL.md",
+            f"---\nname: {skill_name}\ndescription: Demo skill\n---\n\n# {skill_name}\n\n{extra}",
+        )
+    return path
+
+
+def _install_outcome(storage, archive: Path) -> str:
+    try:
+        storage.install_skill_from_archive(archive)
+    except SkillSecurityScanError:
+        return "blocked"
+    return "installed"
+
+
+class TestInstallScanConfigParity:
+    """``ainstall_skill_from_archive`` resolves ``skill_scan.enabled`` from the
+    storage's own ``app_config`` — the same config the archive preflight it
+    inherits already reads — not from the process-global ``get_app_config()``.
+
+    The divergence is reachable whenever the storage outlives a config edit:
+    ``DeerFlowClient`` snapshots ``get_app_config()`` at construction, and the
+    Gateway hands its per-request ``get_config()`` to
+    ``get_or_new_user_skill_storage``, while ``get_app_config`` hot-reloads a
+    later edit of ``config.yaml``. Resolving the content scan from the process
+    global therefore let the two gates of a single install disagree.
+    """
+
+    @pytest.fixture(autouse=True)
+    def _allow_llm_scan(self, monkeypatch):
+        """The per-file LLM scan is not gated by the kill switch; only the
+        static scan is. Stub it so the outcome reflects config resolution."""
+
+        async def _allow(*args, **kwargs):
+            return ScanResult(decision="allow", reason="ok")
+
+        monkeypatch.setattr("deerflow.skills.installer.scan_skill_content", _allow)
+
+    def _local_storage(self, skills_root: Path, config: SimpleNamespace) -> LocalSkillStorage:
+        return LocalSkillStorage(host_path=str(skills_root), app_config=config)
+
+    def _user_storage(self, base: Path, skills_root: Path, config: SimpleNamespace) -> UserScopedSkillStorage:
+        with patch("deerflow.config.paths.get_paths", return_value=Paths(base_dir=base)):
+            with patch("deerflow.config.paths._paths", None):
+                return UserScopedSkillStorage("test-user", host_path=str(skills_root), app_config=config)
+
+    def test_content_scan_honours_the_storages_own_config(self, tmp_path: Path, monkeypatch) -> None:
+        """Storage config ON, process global OFF: the archive must still be blocked."""
+        monkeypatch.setattr("deerflow.config.get_app_config", lambda: SimpleNamespace(skill_scan=SimpleNamespace(enabled=False)))
+
+        skills_root = _skills_root(tmp_path)
+        archive = _malicious_archive(tmp_path)
+        config = _storage_config(skills_root, scan_enabled=True)
+
+        assert _install_outcome(self._local_storage(skills_root, config), archive) == "blocked"
+        assert _install_outcome(self._user_storage(tmp_path, skills_root, config), archive) == "blocked"
+
+    def test_kill_switch_off_in_the_storages_own_config_skips_both_gates(self, tmp_path: Path, monkeypatch) -> None:
+        """Storage config OFF, process global ON: both storages accept it.
+
+        This direction pins the same resolution from the other side — an
+        operator who disables ``skill_scan`` in the config the storage was
+        handed gets no scan, regardless of what the hot-reloaded global says.
+        """
+        monkeypatch.setattr("deerflow.config.get_app_config", lambda: SimpleNamespace(skill_scan=SimpleNamespace(enabled=True)))
+
+        skills_root = _skills_root(tmp_path)
+        archive = _malicious_archive(tmp_path)
+        config = _storage_config(skills_root, scan_enabled=False)
+
+        assert _install_outcome(self._local_storage(skills_root, config), archive) == "installed"
+        assert _install_outcome(self._user_storage(tmp_path, skills_root, config), archive) == "installed"
+
+    def test_llm_scan_receives_the_storages_own_config(self, tmp_path: Path, monkeypatch) -> None:
+        """The per-file LLM scan resolves its config the same way.
+
+        ``skill_scan.enabled`` does not gate it — ``scan_skill_content`` always
+        runs — but it reads ``skill_evolution.moderation_model_name`` and builds
+        the model from its ``app_config``, so leaving it on the process global
+        keeps the two halves of one install on two different configs.
+        """
+        seen: list[object] = []
+
+        async def _record(*args, **kwargs):
+            seen.append(kwargs.get("app_config"))
+            return ScanResult(decision="allow", reason="ok")
+
+        monkeypatch.setattr("deerflow.skills.installer.scan_skill_content", _record)
+
+        skills_root = _skills_root(tmp_path)
+        archive = _archive(tmp_path)
+        config = _storage_config(skills_root, scan_enabled=True)
+
+        self._local_storage(skills_root, config).install_skill_from_archive(archive)
+        self._user_storage(tmp_path, skills_root, config).install_skill_from_archive(archive)
+
+        assert seen, "the LLM scan should have run for the installed SKILL.md"
+        assert all(entry is config for entry in seen), "every LLM scan must receive the storage's own app_config"